Fix running native aot testing locally#125354
Fix running native aot testing locally#125354MichalStrehovsky wants to merge 3 commits intodotnet:mainfrom
Conversation
Running the native AOT tests locally was broken in dotnet#120949 ``` Build FAILED. d:\git\runtime4\src\tests\Common\tests.targets(53,5): error MSB3030: Could not copy the file "d:\git\runtime4\artifacts\tests\coreclr\windows.x64.Checked\nativeaot\CustomMainWithStubExe\CustomMainWithStubExe\CustomMainWithStubExe.testResults.xml" because it was not found. d:\git\runtime4\src\tests\Common\tests.targets(53,5): error MSB3030: Could not copy the file "d:\git\runtime4\artifacts\tests\coreclr\windows.x64.Checked\nativeaot\SmokeTests\PInvoke\PInvoke\PInvoke.testResults.xml" because it was not found. d:\git\runtime4\src\tests\Common\tests.targets(53,5): error MSB3030: Could not copy the file "d:\git\runtime4\artifacts\tests\coreclr\windows.x64.Checked\nativeaot\SmokeTests\AttributeTrimming\AttributeTrimming\AttributeTrimming.testResults.xml" because it was not found. ``` repeated many times.
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Pull request overview
Updates the NativeAOT test subtree to use a single merged test wrapper project for local runs, instead of treating each NativeAOT test project as its own merged runner (which was causing missing *.testResults.xml artifacts during local execution).
Changes:
- Add
src/tests/nativeaot/nativeaot.csprojas a merged test runner wrapper referencing NativeAOT test projects. - Remove NativeAOT subtree-wide “treat everything as merged runner / has merged-in tests” behavior (
Directory.Build.targets+HasMergedInTestsprops). - Remove
HasMergedInTestsfromStartupHook.csprojto align with the new wrapper approach.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/tests/nativeaot/nativeaot.csproj | New merged wrapper project that references NativeAOT test projects and imports MergedTestRunner.targets. |
| src/tests/nativeaot/StartupHook/StartupHook.csproj | Removes HasMergedInTests from this test project. |
| src/tests/nativeaot/Directory.Build.targets | Deletes subtree-wide import that made each NativeAOT project a merged runner. |
| src/tests/nativeaot/Directory.Build.props | Removes subtree-wide HasMergedInTests property. |
You can also share your feedback on Copilot code review. Take the survey.
|
The only runtime tests we run for NativeAOT on iOS/tvOS in CI are the nativeaot/SmokeTests tree. This change makes those runs effectively no-op as all tests will be skipped. Can we condition these changes on |
|
The /nativeaot/ tests are mostly tests for compiler behaviors and CoreLib. They don't exercise much platform logic. I don't actually remember last time we hit an issue in these that would be OS/platform specific (if we hit an OS/platform issue in any of these, usually all the tests fail on it; there are small exceptions, like the executable file size test that we don't run on mobile anyway). E.g. it's possible we only trigger a couple GCs as part of the entire test run because none of these tests are GC tests. We're also on track to delete the /nativeaot/ coverage that is duplicated elsewhere. You deleted the ComWrappers test recently. I think the PInvoke.cs and Threading.cs should follow (Threading.cs even has a TODO for it). #121895 (comment) was another discussion about this. For mobile testing it's a lot more value to run the libs tests because that's where all the iOS specific behaviors get exercised (Apple crypto and stuff like that). But to ship, we need to be able to run the full suite. If we can't run RequiresProcessIsolation tests, that's a test gap. The test gap is a general gap, not specific to native AOT subtree or even not specific to native AOT. We may need to live with the gap or solve it in the test infra. |
|
If we need to find a subset of /src/tests that randomly checks "does native AOT work at all", we can e.g. use the Loader subset. |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Most of the Loader tests are not RequiresProcessIsolation but it seems like we still don't end up with anything runnable. Are the only tests the mobile infra can run those that are marked If we're fine with close to zero coverage on mobile (as we were when we decided we'll only run the nativeaot subtree I guess), I can add a We need to run the entire /src/tests/ tree and libs tests to ship, that's the team's agreed upon quality gate. That's the tests that exercise the GC, synchronization primitives, crypto APIs etc. that may have iOS-specific issues. The native AOT subtree is not likely to find anything - the compiler doesn't meaningfully distinguish between iOS and macOS and neither do the tested features like reflection and type loading. |
|
It should run merged test runners without needing Don't we have a special logic path for thunks that's only used on platforms where we can't change page permissions? Or do we use that one as well on macOS? If we really don't have any specific runtime code, I'm fine dropping the runtime tests (the infra is a hassle as you've seen). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
Yup, for Marshal.GetFunctionPointerForDelegate. We test that in the Interop subtree and System.Runtime.InteropServices libraries tests. The coverage in PInvoke.cs is redundant and is going to be deleted. This is the reason why our native aot innerloop testing runs the nativeaot;Loader;Interop;async trees, not just nativeaot. Outerloop runs everything.
It doesn't work per the previous CI results. I've just added a mobile smoke test with HasMergedInTests that does a NullRef and catches it to approximate the "does it work at all?" coverage we were previously getting. We need tests that call into OS and exercise OS APIs. The nativeaot subtree is as good at it as any other subtree. Our mobile testing strategy is "cross our fingers hoping macOS testing covers this" and has been successful so far. #81075 has been open for 3+ years and we don't even have a bug to run pri0-pri1 runtime tests. Cc @vitek-karas |
Ah, take that back, we don't have coverage for it in SmokeTests at all, so we're not even losing that. |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
If we don't have coverage of that currently I'm totally fine just deleting the jobs from CI entirely. |
I'm fine either way too. My motivation for the one test was to ensure we don't regress the "we can run at least something with native AOT using the test infra" (even though it's only a The desired state is that we run all libs tests and pri0 and pri1 tests (the libs tests feeling as higher priority), but I don't know what are the plans for that. @vitek-karas to make a call if we want to keep a test or just delete the leg for now. |
Fixes #125317