-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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 53 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,79 @@ | |
| </ItemGroup> | ||
| </Target> | ||
|
|
||
| <Target Name="ExcludeExistingR2RBinaries" | ||
|
Member
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:
Member
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> | ||
|
|
||
| <Target Name="AddExistingR2RBinariesReferencesForCrossgen2" | ||
| Condition="'$(TestReadyToRun)' == 'true'" | ||
| AfterTargets="_PrepareForReadyToRunCompilation"> | ||
| <ItemGroup> | ||
| <_ReadyToRunAssembliesToReference Include="@(_BundleAssembliesToCopy)" /> | ||
| </ItemGroup> | ||
| </Target> | ||
|
|
||
| <Target Name="RemoveDbgBinsFromTestR2ROutput" | ||
| Condition="'$(TestReadyToRun)' == 'true'" | ||
| BeforeTargets="_CopyFilesMarkedCopyLocal"> | ||
| <ItemGroup> | ||
| <ReferenceCopyLocalPaths | ||
| Remove="@(ReferenceCopyLocalPaths->WithMetadataValue('Extension', '.dbg'))" /> | ||
| </ItemGroup> | ||
| </Target> | ||
|
|
||
| <Target Name="RemoveDbgBinsFromTestR2RPublish" | ||
| Condition="'$(TestReadyToRun)' == 'true'" | ||
| BeforeTargets="_CopyResolvedFilesToPublishPreserveNewest"> | ||
| <ItemGroup> | ||
| <_ResolvedFileToPublishPreserveNewest | ||
| Remove="@(_ResolvedFileToPublishPreserveNewest->WithMetadataValue('Extension', '.dbg'))" /> | ||
| </ItemGroup> | ||
| </Target> | ||
|
|
||
| <Target Name="CopyExistingR2RBinaries" | ||
| Condition="'$(TestReadyToRun)' == 'true'" | ||
| AfterTargets="_CopyResolvedFilesToPublishAlways"> | ||
|
|
||
| <Copy SourceFiles="@(_BundleAssembliesToCopy)" | ||
| DestinationFolder="$(PublishDir)" | ||
| OverwriteReadOnlyFiles="$(OverwriteReadOnlyFiles)" | ||
| Retries="$(CopyRetryCount)" | ||
| RetryDelayMilliseconds="$(CopyRetryDelayMilliseconds)" | ||
| UseHardlinksIfPossible="$(CreateHardLinksForPublishFilesIfPossible)" | ||
| UseSymboliclinksIfPossible="$(CreateSymbolicLinksForPublishFilesIfPossible)" /> | ||
|
|
||
| </Target> | ||
|
|
||
| <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'"> | ||
|
Member
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.
Contributor
Author
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
Member
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
Member
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
Member
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.
Member
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?
Member
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.
Member
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.
Member
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'"> | ||
| <TestReadyToRunProjects Include="$(MSBuildThisFileDirectory)System.*/tests/**/*.Tests.csproj" /> | ||
|
|
||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.ComponentModel.Composition/tests/System.ComponentModel.Composition.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Composition/tests/System.Composition.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Console/tests/System.Console.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Dynamic.Runtime.Tests/System.Dynamic.Runtime.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.IO.FileSystem.Watcher/tests/System.IO.FileSystem.Watcher.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Reflection.Tests/System.Reflection.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Reflection.TypeExtensions/tests/System.Reflection.TypeExtensions.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Runtime.Extensions.Tests/System.Runtime.Extensions.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Runtime.Loader/tests/DefaultContext/System.Runtime.Loader.DefaultContext.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Text.RegularExpressions/tests/FunctionalTests/System.Text.RegularExpressions.Tests.csproj" /> | ||
| <TestReadyToRunProjects Remove="$(MSBuildThisFileDirectory)System.Threading.Thread/tests/System.Threading.Thread.Tests.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Condition="'$(RunSmokeTestsOnly)' == 'true'" | ||
| Include="@(SmokeTestProject)" /> | ||
|
|
@@ -605,9 +630,16 @@ | |
| BuildInParallel="false" /> | ||
| <ProjectReference Condition="'$(RunGrpcTestsOnly)' == 'true'" | ||
| Include="@(GrpcTestProject)" /> | ||
| <ProjectReference Condition="'$(TestReadyToRun)' == 'true'" | ||
| Include="@(TestReadyToRunProjects)" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(RunSmokeTestsOnly)' != 'true' and '$(RunGrpcTestsOnly)' != 'true' and '$(RunHighAOTResourceRequiringTestsOnly)' != 'true' and '$(RunLimitedSetOfTests)' != 'true'"> | ||
| <ItemGroup Condition="'$(RunSmokeTestsOnly)' != 'true' | ||
|
Member
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
Contributor
Author
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
Member
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
Contributor
Author
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' | ||
| and '$(TestReadyToRun)' != 'true'"> | ||
|
|
||
| <ProjectReference Include="$(MSBuildThisFileDirectory)*\tests\**\*.Tests.csproj" | ||
| Exclude="@(ProjectExclusions)" | ||
| Condition="'$(TestAssemblies)' == 'true'" | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.