-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Remove RequiresProcessIsolation from HFA tests
#111406
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
Conversation
Tests using `CMakeProjectReference` seem to work just fine with the merged runner.
Before (CoreCLR, checked, Ryzen 9700X):
```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
47.303 | 112 | 112 | 0 | 0 | JIT.jit64.jit64_1
```
After:
```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
2.694 | 112 | 112 | 0 | 0 | JIT.jit64.jit64_1
```
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume none of the projects define custom ennvars in csproj? I remember those required process isolation too (the tests still pass without the envvars, but may just stop being useful)
Nope, these all look the same (a couple project references and settings related to opt/debug info and that's it). |
As per dotnet#111406, `RequiresProcessIsolation` is not needed just because of `CMakeProjectReference`. The tests also have `CLRTestTargetUnsupported`, but all of them do, so I just moved it to the merged runner. (Note, mcc_i00 didn't have CLRTestTargetUnsupported to make it Windows only, but instead we had it in issues.targets as `needs triage`, so deleting that.) Before (CoreCLR checked, Microsoft DevBox): ``` Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary ============================================================================ 74.311 | 56 | 56 | 0 | 0 | JIT.jit64.jit64_2 ``` After: ``` Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary ============================================================================ 2.088 | 56 | 56 | 0 | 0 | JIT.jit64.jit64_2 ```
As per #111406, `RequiresProcessIsolation` is not needed just because of `CMakeProjectReference`. The tests also have `CLRTestTargetUnsupported`, but all of them do, so I just moved it to the merged runner. (Note, mcc_i00 didn't have CLRTestTargetUnsupported to make it Windows only, but instead we had it in issues.targets as `needs triage`, so deleting that.) Before (CoreCLR checked, Microsoft DevBox): ``` Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary ============================================================================ 74.311 | 56 | 56 | 0 | 0 | JIT.jit64.jit64_2 ``` After: ``` Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary ============================================================================ 2.088 | 56 | 56 | 0 | 0 | JIT.jit64.jit64_2 ```
Tests using
CMakeProjectReferenceseem to work just fine with the merged runner.Before (CoreCLR, checked, Ryzen 9700X):
After:
Cc @dotnet/jit-contrib @jkoritzinsky