Skip to content
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

Clean up testgrouping.proj and move JIT/Intrinsics under JIT.1 #96333

Merged
merged 3 commits into from
Dec 28, 2023

Conversation

trylek
Copy link
Member

@trylek trylek commented Dec 27, 2023

In my attempts to switch over the remainder of JIT tests to the merged wrapper model I hit a weird bug in two tests, JIT/Intrinsics/TypeIntrinsics_r.csproj and TypeIntrinsics_ro.csproj when running under CG2. I found out that the bug is real even without test merging, it was just hidden due to a weird corner case in the script helixpublishwitharcade.proj:

The testgrouping.proj script is used for two purposes - to split some large test groups using the TestProjects item group and to merge multiple legacy test XUnit wrappers for small test groups into a single Helix work item using the XUnitWrapperGrouping item group.

As I found out, the somewhat hacky logic in helixpublishwitharcade.proj doesn't directly use the above item groups to construct the Helix work items, it uses a roundabout way of doing that which among others states that the "TestGroup" trait used to filter tests within a single legacy XUnit wrapper at the XUnit console level was only set when there was a single XUnit wrapper in a Helix work item.

As it turns out, the PayloadGroup "JIT.2" included two subtrees - the "JIT/Intrinsics" folder and the "JIT/SIMD" subtree. Once the JIT/SIMD subtree was converted to the merged test wrapper model, the JIT/Intrinsics/JIT.Intrinsics.XUnitWrapper.dll remained as the only (legacy) wrapper in the JIT.2 group, thus triggering the logic in helixpublishwitharcade that added the "-trait TestGroup=JIT.2" to the xunit.console execution command line, effectively blocking out all 31 tests in that wrapper.

In the first iteration I have only moved the JIT/Intrinsics wrapper under the PayloadGroup "JIT.1" and I have deleted all items referring to tests that have already been merged. My plan is to use this first iteration to verify that runtime-coreclr2 outerloop now fails as it should (because the two problematic TypeIntrinsics tests should start running in CG2 mode with the change).

I'll then publish an update to the PR adding an issues.targets blocking clause for the two TypeIntrinsics tests and I'll clean up helixpublishwitharcade.proj by removing the no longer needed logic around TestGroup as all test groups using the TestGrouping item group have already been converted to the merged model.

Thanks

Tomas

In my attempts to switch over the remainder of JIT tests to the
merged wrapper model I hit a weird bug in two tests,
JIT/Intrinsics/TypeIntrinsics_r.csproj and TypeIntrinsics_ro.csproj
when running under CG2. I found out that the bug is real even without
test merging, it was just hidden due to a weird corner case in the
script helixpublishwitharcade.proj:

The testgrouping.proj script is used for two purposes - to split
some large test groups using the TestProjects item group and to
merge multiple legacy test XUnit wrappers for small test groups
into a single Helix work item using the XUnitWrapperGrouping item
group.

As I found out, the somewhat hacky logic in helixpublishwitharcade.proj
doesn't directly use the above item groups to construct the Helix
work items, it uses a roundabout way of doing that which among others
states that the "TestGroup" trait used to filter tests within a
single legacy XUnit wrapper at the XUnit console level was only
set when there was a single XUnit wrapper in a Helix work item.

As it turns out, the PayloadGroup "JIT.2" included two subtrees -
the "JIT/Intrinsics" folder and the "JIT/SIMD" subtree. Once the
JIT/SIMD subtree was converted to the merged test wrapper model,
the JIT/Intrinsics/JIT.Intrinsics.XUnitWrapper.dll remained as the
only (legacy) wrapper in the JIT.2 group, thus triggering the
logic in helixpublishwitharcade that added the "-trait TestGroup=JIT.2"
to the xunit.console execution command line, effectively blocking
out all 31 tests in that wrapper.

In the first iteration I have only moved the JIT/Intrinsics wrapper
under the PayloadGroup "JIT.1" and I have deleted all items referring
to tests that have already been merged. My plan is to use this first
iteration to verify that runtime-coreclr2 outerloop now fails as it
should (because the two problematic TypeIntrinsics tests should start
running in CG2 mode with the change).

I'll then publish an update to the PR adding an issues.targets
blocking clause for the two TypeIntrinsics tests and I'll clean up
helixpublishwitharcade.proj by removing the no longer needed logic
around TestGroup as all test groups using the TestGrouping item group
have already been converted to the merged model.

Thanks

Tomas
@ghost
Copy link

ghost commented Dec 27, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

In my attempts to switch over the remainder of JIT tests to the merged wrapper model I hit a weird bug in two tests, JIT/Intrinsics/TypeIntrinsics_r.csproj and TypeIntrinsics_ro.csproj when running under CG2. I found out that the bug is real even without test merging, it was just hidden due to a weird corner case in the script helixpublishwitharcade.proj:

The testgrouping.proj script is used for two purposes - to split some large test groups using the TestProjects item group and to merge multiple legacy test XUnit wrappers for small test groups into a single Helix work item using the XUnitWrapperGrouping item group.

As I found out, the somewhat hacky logic in helixpublishwitharcade.proj doesn't directly use the above item groups to construct the Helix work items, it uses a roundabout way of doing that which among others states that the "TestGroup" trait used to filter tests within a single legacy XUnit wrapper at the XUnit console level was only set when there was a single XUnit wrapper in a Helix work item.

As it turns out, the PayloadGroup "JIT.2" included two subtrees - the "JIT/Intrinsics" folder and the "JIT/SIMD" subtree. Once the JIT/SIMD subtree was converted to the merged test wrapper model, the JIT/Intrinsics/JIT.Intrinsics.XUnitWrapper.dll remained as the only (legacy) wrapper in the JIT.2 group, thus triggering the logic in helixpublishwitharcade that added the "-trait TestGroup=JIT.2" to the xunit.console execution command line, effectively blocking out all 31 tests in that wrapper.

In the first iteration I have only moved the JIT/Intrinsics wrapper under the PayloadGroup "JIT.1" and I have deleted all items referring to tests that have already been merged. My plan is to use this first iteration to verify that runtime-coreclr2 outerloop now fails as it should (because the two problematic TypeIntrinsics tests should start running in CG2 mode with the change).

I'll then publish an update to the PR adding an issues.targets blocking clause for the two TypeIntrinsics tests and I'll clean up helixpublishwitharcade.proj by removing the no longer needed logic around TestGroup as all test groups using the TestGrouping item group have already been converted to the merged model.

Thanks

Tomas

Author: trylek
Assignees: -
Labels:

area-Infrastructure

Milestone: -

@ghost ghost assigned trylek Dec 27, 2023
@trylek
Copy link
Member Author

trylek commented Dec 27, 2023

/azp run runtime-coreclr crossgen2

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@trylek
Copy link
Member Author

trylek commented Dec 27, 2023

/azp run runtime-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@trylek
Copy link
Member Author

trylek commented Dec 27, 2023

/azp run runtime-coreclr crossgen2

@trylek
Copy link
Member Author

trylek commented Dec 27, 2023

/azp run runtime-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Add issues.targets entries blocking out the tests
JIT/Intrinsics/TypeIntrinsics_r[o].csproj that currently fail in
CG2 mode. I have also cleaned up the helixpublishwitharcade.proj
script by removing parts pertaining to the no longer used
TestGrouping logic.

Thanks

Tomas
@trylek
Copy link
Member Author

trylek commented Dec 27, 2023

/azp run runtime-coreclr crossgen2

@trylek
Copy link
Member Author

trylek commented Dec 27, 2023

/azp run runtime-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@trylek trylek merged commit 1fa3f1d into dotnet:main Dec 28, 2023
189 of 192 checks passed
@trylek trylek deleted the TestGroupingCleanupAndFix branch December 28, 2023 19:09
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants