-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Restore correct outerloop priority tracking and test execution #123112
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
When this was originally written, we set a Priority property based on the __Priority environment variable as well as CLRTestPriorityToBuild. At some point, we lost this. As a result, we have been missing test coverage in our outerloops. This restores the correct outerloop test execution behavior.
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Optimistically marking as auto-merge. Since BA tracks the outerloop pipeline, that will get this in faster if we don't have any regressions (which I think is likely). |
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.
Pull request overview
This PR restores correct outerloop test execution by fixing the priority tracking mechanism in the XUnit wrapper generator. The original implementation used a generic Priority property that was lost at some point, causing outerloop tests to not execute properly. This change unifies on the CLRTestPriorityToBuild property, which is the established MSBuild property used throughout the test infrastructure.
Changes:
- Replaced the
Prioritycompiler-visible property withCLRTestPriorityToBuildto align with the MSBuild property name - Updated the helper method from
Priority()toCLRTestPriorityToBuild()to match the new property name - Fixed outerloop test filtering logic to use the correct property
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.props | Updated compiler-visible property from Priority to CLRTestPriorityToBuild |
| src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs | Updated outerloop attribute filtering to check CLRTestPriorityToBuild() instead of Priority() |
| src/tests/Common/XUnitWrapperGenerator/OptionsHelper.cs | Renamed constant and helper method from Priority to CLRTestPriorityToBuild to match the MSBuild property |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/ba-g failure is #123195 |
#123112 started running more outerloop tests, including: https://github.com/dotnet/runtime/blob/81e28e28a7dd0c5109f8a3c0e458d3de577e78f8/src/tests/JIT/Methodical/flowgraph/dev10_bug679008/sealedCastVariance.cs#L13-L32 We were incorrectly optimizing the `a.GetType() == typeof(Action<object>)` check to `false` (type of the parameter is `Action<string>` and JIT was asking for exact classes of `Action<string>`; and there are none).
Test started running with #123112
Test started running with #123112 This is past our cutoff so the method body gets compiled to `TypeLoadException`. We have generic recursion coverage in native AOT tests. Cc @dotnet/ilc-contrib
…#123479) Test started running with #123112 Cc @dotnet/ilc-contrib
#123112 started running more outerloop tests, including: https://github.com/dotnet/runtime/blob/81e28e28a7dd0c5109f8a3c0e458d3de577e78f8/src/tests/JIT/Methodical/flowgraph/dev10_bug679008/sealedCastVariance.cs#L13-L32 We were incorrectly optimizing the `a.GetType() == typeof(Action<object>)` check to `false` (type of the parameter is `Action<string>` and JIT was asking for exact classes of `Action<string>`; and there are none). Cc @dotnet/ilc-contrib @EgorBo
Test started running with #123112 This is past our cutoff so the method body gets compiled to `TypeLoadException`. We have generic recursion coverage in native AOT tests.
Test started running with #123112 Cc @dotnet/ilc-contrib
Test started running with #123112 Cc @dotnet/ilc-contrib
Test started running with #123112 Cc @dotnet/ilc-contrib
These were all enabled in dotnet#123112
These were all enabled in #123112
When this was originally written, we set a Priority property based on the __Priority environment variable for the generator as well as CLRTestPriorityToBuild for the overall build.
At some point, we lost this. As a result, we have been missing test coverage in our outerloops.
This restores the correct outerloop test execution behavior by unifying on CLRTestPriorityToBuild.