-
Notifications
You must be signed in to change notification settings - Fork 5k
Add new Pipeline for Running Libs Tests with TestReadyToRun #91229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
58471ac
d424419
cbae83d
bbcbda5
74c393d
f39ec24
e76d7b2
800bf85
4888506
f95be94
ce0beb7
084dcd2
d6145d9
7a98a59
57a1ed0
9c9845c
201c744
0e5318e
94b209d
baf03ff
bd083f9
70a5310
29bdc40
0dafa7a
0987abf
c050621
21f95ef
59af6bb
52875a7
43eee05
4666ac0
79a9028
e95e20d
e857281
d193ab5
7a3a1d5
cd94d5a
3c54a84
879e5ea
11cdb6b
19bb85f
81cb9e3
47760b3
0728170
4262460
403a5f9
797f2c0
048d35e
9939472
033508d
54088ef
7a23788
7c97aa7
f54af82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,10 @@ | |
<DefineConstants>$(DefineConstants);SINGLE_FILE_TEST_RUNNER</DefineConstants> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition="'$(TestReadyToRun)' == 'true'"> | ||
<DefineConstants>$(DefineConstants);TEST_READY_TO_RUN_COMPILED</DefineConstants> | ||
</PropertyGroup> | ||
|
||
<Import Project="$(CoreCLRBuildIntegrationDir)Microsoft.DotNet.ILCompiler.SingleEntry.targets" Condition="'$(TestNativeAot)' == 'true'" /> | ||
|
||
<ItemGroup Condition="'$(TestNativeAot)' == 'true'"> | ||
|
@@ -106,6 +110,114 @@ | |
</ItemGroup> | ||
</Target> | ||
|
||
<!-- | ||
For TestReadyToRun, we need the whole framework to be R2R-compiled besides | ||
the actual test assembly. However, this is a very lengthy process and it's | ||
unnecessary in this case because we already have an R2R-compiled framework. | ||
So, we have to tell the build that we already have these binaries so that it | ||
doesn't build them again for each test. | ||
--> | ||
<Target Name="ExcludeExistingR2RBinaries" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please describe these targets via xml comments. I don't understand what the following targets are doing or why we need them so a comment would be appreciated: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @ViktorHofer here - copies like this can cause extra work and even create incremental build issues when they use wildcards like this. It'd be good to understand them. |
||
Condition="'$(TestReadyToRun)' == 'true'" | ||
BeforeTargets="_PrepareForReadyToRunCompilation"> | ||
<PropertyGroup> | ||
<ArtifactsNetCoreAppBundlePath>$(ArtifactsObjDir)Microsoft.NETCore.App.Bundle/</ArtifactsNetCoreAppBundlePath> | ||
<ArtifactsNetCoreAppBundlePath>$(ArtifactsNetCoreAppBundlePath)$(Configuration)/$(NetCoreAppCurrent)/$(OutputRID)/output/</ArtifactsNetCoreAppBundlePath> | ||
<ArtifactsNetCoreAppBundlePath>$(ArtifactsNetCoreAppBundlePath)shared/$(MicrosoftNetCoreAppFrameworkName)/$(PackageVersion)/</ArtifactsNetCoreAppBundlePath> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<_BundleAssembliesToCopy Include="$(ArtifactsNetCoreAppBundlePath)*.dll" /> | ||
<ResolvedFileToPublish Remove="@(_BundleAssembliesToCopy)" MatchOnMetadata="Filename" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<!-- | ||
For TestReadyToRun, each crossgen'd assembly needs to reference the whole | ||
framework. For this, it looks at the contents of the same list that contains | ||
all the assemblies we're going to R2R-compile. However, since we removed those | ||
belonging to the framework we have ready to use in the previous target, then | ||
the references list generated in _PrepareForReadyToRunCompilation is incomplete. | ||
So, we add those missing assemblies only to the references list in this target. | ||
--> | ||
<Target Name="AddExistingR2RBinariesReferencesForCrossgen2" | ||
Condition="'$(TestReadyToRun)' == 'true'" | ||
AfterTargets="_PrepareForReadyToRunCompilation"> | ||
<ItemGroup> | ||
<_ReadyToRunAssembliesToReference Include="@(_BundleAssembliesToCopy)" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<!-- | ||
For TestReadyToRun, debugging binaries bloat the test sizes way too much and | ||
makes the Helix machines run out of disk. Since we don't need them for the | ||
TestReadyToRun test runs, we remove them from the list that is later on copied | ||
to the final location. | ||
--> | ||
<Target Name="RemoveDbgBinsFromTestR2ROutput" | ||
Condition="'$(TestReadyToRun)' == 'true'" | ||
BeforeTargets="_CopyFilesMarkedCopyLocal"> | ||
<ItemGroup> | ||
<ReferenceCopyLocalPaths | ||
Remove="@(ReferenceCopyLocalPaths->WithMetadataValue('Extension', '.dbg'))" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<!-- | ||
Very similarly to the previous target, we need to get rid of the debugging | ||
binaries from the publishing directory as well. | ||
--> | ||
<Target Name="RemoveDbgBinsFromTestR2RPublish" | ||
Condition="'$(TestReadyToRun)' == 'true'" | ||
BeforeTargets="_CopyResolvedFilesToPublishPreserveNewest"> | ||
<ItemGroup> | ||
<_ResolvedFileToPublishPreserveNewest | ||
Remove="@(_ResolvedFileToPublishPreserveNewest->WithMetadataValue('Extension', '.dbg'))" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<!-- | ||
As explained in Target 'ExcludeExistingR2RBinaries' up above, for TestReadyToRun | ||
we need the fully R2R-compiled framework, but we already have it elsewhere. So, | ||
once the test's specific stuff is constructed, we copy the R2R-compiled framework | ||
to the test's self-contained directory so the test can use it when called. | ||
--> | ||
<Target Name="CopyExistingR2RBinaries" | ||
Condition="'$(TestReadyToRun)' == 'true'" | ||
AfterTargets="_CopyResolvedFilesToPublishAlways"> | ||
|
||
<Copy SourceFiles="@(_BundleAssembliesToCopy)" | ||
DestinationFolder="$(PublishDir)" | ||
OverwriteReadOnlyFiles="$(OverwriteReadOnlyFiles)" | ||
Retries="$(CopyRetryCount)" | ||
RetryDelayMilliseconds="$(CopyRetryDelayMilliseconds)" | ||
UseHardlinksIfPossible="$(CreateHardLinksForPublishFilesIfPossible)" | ||
UseSymboliclinksIfPossible="$(CreateSymbolicLinksForPublishFilesIfPossible)" /> | ||
|
||
</Target> | ||
|
||
<!-- | ||
There are a few tests that need a 'live-ref-pack', which is missing from the | ||
publish directory in TestReadyToRun builds. This target copies it there. | ||
--> | ||
<Target Name="CopyLiveRefPackIfPresent" | ||
Condition="'$(TestReadyToRun)' == 'true'" | ||
AfterTargets="CopyExistingR2RBinaries"> | ||
|
||
<ItemGroup> | ||
<OutDirLiveRefPackFiles Include="$(OutDir)live-ref-pack/*" /> | ||
</ItemGroup> | ||
|
||
<Copy SourceFiles="@(OutDirLiveRefPackFiles)" | ||
DestinationFolder="$(PublishDir)live-ref-pack" | ||
OverwriteReadOnlyFiles="$(OverwriteReadOnlyFiles)" | ||
Retries="$(CopyRetryCount)" | ||
RetryDelayMilliseconds="$(CopyRetryDelayMilliseconds)" | ||
UseHardlinksIfPossible="$(CreateHardLinksForPublishFilesIfPossible)" | ||
UseSymboliclinksIfPossible="$(CreateSymbolicLinksForPublishFilesIfPossible)" /> | ||
|
||
</Target> | ||
|
||
<Target Name="__UpdateExcludedAssembliesFromSingleFile" | ||
Inputs="ExcludeFromSingleFile" | ||
Outputs="ResolvedFileToPublish" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,10 @@ | |
<EnableCoverageSupport Condition="'$(ContinuousIntegrationBuild)' != 'true'">true</EnableCoverageSupport> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition="'$(TestReadyToRun)' == 'true'"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's zero references to TestReadyToRun in src/libraries. Should all of TestReadyToRun handling stay in eng/? tests.singlefile.targets looks more suitable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried moving it, but then the build fails in other places because it can't find the
In this case, this means |
||
<UseLocalAppHostPack>true</UseLocalAppHostPack> | ||
</PropertyGroup> | ||
|
||
<!-- To enable the interpreter for mono desktop, we need to pass an env switch --> | ||
<PropertyGroup> | ||
<MonoEnvOptions Condition="'$(MonoEnvOptions)' == '' and '$(TargetsMobile)' != 'true' and '$(MonoForceInterpreter)' == 'true'">--interpreter</MonoEnvOptions> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,14 @@ | |
<ProjectReference Include="$(MonoProjectRoot)wasm\symbolicator\WasmSymbolicator.csproj" Condition="'$(TargetOS)' == 'browser'" /> | ||
|
||
<!-- needed to test workloads for wasm --> | ||
<ProjectReference Include="$(InstallerProjectRoot)pkg\sfx\Microsoft.NETCore.App\Microsoft.NETCore.App.Runtime.sfxproj" Pack="true" Condition="'$(TargetOS)' == 'browser' or '$(TargetOS)' == 'wasi'" /> | ||
<ProjectReference Include="$(InstallerProjectRoot)pkg\sfx\Microsoft.NETCore.App\Microsoft.NETCore.App.Runtime.sfxproj" | ||
Pack="true" | ||
Condition="'$(TargetOS)' == 'browser' or '$(TargetOS)' == 'wasi'" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(TestReadyToRun)' == 'true'"> | ||
<ProjectReference Include="$(InstallerProjectRoot)pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.sfxproj" /> | ||
<ProjectReference Include="$(InstallerProjectRoot)pkg/sfx/bundle/Microsoft.NETCore.App.Bundle.bundleproj" /> | ||
Comment on lines
+35
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is that required? I see that workload tests only have the ProjectReference with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how workload tests work, but I wouldn't be amazed if they need the NuGet packages or the installers or the archives. This is another option that I built into the Shared Framework SDK to make it easier to dump the layout to disk wherever you want and not having to guess where the output will be. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. What's the default output location of the bundle project? Is it put to disk somewhere without invoking the target? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Shared Framework SDK by default doesn't actually lay it out on disk. The Archives and Installers NuGet packages from dotnet/arcade use this target to put it into the intermediate output path for the project and then use that to produce their output. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. For the sake of static graph (which disallows calling custom targets in a P2P protocol), can we enable a switch so that the target is automatically sequenced into the build? The project's output directory would be a good default place. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be able to add the target to the Bundle.bundleproj project and sequence it into the build, but then we still end up building the installers (which is what's causing the build failures that prompted this suggestion). |
||
</ItemGroup> | ||
|
||
<Import Project="$(RepositoryEngineeringDir)testing\wasm-provisioning.targets" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -597,6 +597,31 @@ | |
--> | ||
</ItemGroup> | ||
|
||
<!-- | ||
There is a decent number of hidden tests that fail with TestReadyToRun, and | ||
it's proven to be a neverending task to flush all of them at once. So, we will | ||
start by only enabling the tests we have confirmed work correctly, and we will | ||
gradually enable the rest in subsequent PR's. | ||
|
||
Tracking Issue for this work item: https://github.com/dotnet/runtime/issues/95928 | ||
--> | ||
<ItemGroup Condition="'$(TestReadyToRun)' == 'true'"> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.DependencyInjection/tests/DI.External.Tests/Microsoft.Extensions.DependencyInjection.ExternalContainers.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)Microsoft.XmlSerializer.Generator/tests/Microsoft.XmlSerializer.Generator.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.ComponentModel.Composition/tests/System.ComponentModel.Composition.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Composition/tests/System.Composition.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Console/tests/System.Console.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Dynamic.Runtime.Tests/System.Dynamic.Runtime.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.FileSystem.Watcher/tests/System.IO.FileSystem.Watcher.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Reflection.Tests/System.Reflection.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Reflection.TypeExtensions/tests/System.Reflection.TypeExtensions.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Runtime.Extensions.Tests/System.Runtime.Extensions.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime.Loader/tests/DefaultContext/System.Runtime.Loader.DefaultContext.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Text.RegularExpressions/tests/FunctionalTests/System.Text.RegularExpressions.Tests.csproj" /> | ||
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Threading.Thread/tests/System.Threading.Thread.Tests.csproj" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Condition="'$(RunSmokeTestsOnly)' == 'true'" | ||
Include="@(SmokeTestProject)" /> | ||
|
@@ -607,7 +632,11 @@ | |
Include="@(GrpcTestProject)" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(RunSmokeTestsOnly)' != 'true' and '$(RunGrpcTestsOnly)' != 'true' and '$(RunHighAOTResourceRequiringTestsOnly)' != 'true' and '$(RunLimitedSetOfTests)' != 'true'"> | ||
<ItemGroup Condition="'$(RunSmokeTestsOnly)' != 'true' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not make this more complicated than it already is. The existing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok will change that. Just worth mentioning I thought you all didn't want to change that further because I originally disabled the bad tests with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you point me to the comment asking not to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad, it was about disabling them in |
||
and '$(RunGrpcTestsOnly)' != 'true' | ||
and '$(RunHighAOTResourceRequiringTestsOnly)' != 'true' | ||
and '$(RunLimitedSetOfTests)' != 'true'"> | ||
|
||
<ProjectReference Include="$(MSBuildThisFileDirectory)*\tests\**\*.Tests.csproj" | ||
Exclude="@(ProjectExclusions)" | ||
Condition="'$(TestAssemblies)' == 'true'" | ||
|
Uh oh!
There was an error while loading. Please reload this page.