Rename runner-agnostic SDKCustomXUnit* Helix infrastructure to a runner-neutral name#55099
Conversation
…names Renames the runner-agnostic Helix work-item infrastructure from xUnit-specific names to runner-neutral ones after the xUnit -> MSTest/MTP migration. No functional change. - test/xunit-runner/ -> test/test-runner/ - XUnitRunner.targets/XUnitPublish.targets -> TestRunner.targets/TestPublish.targets - SDKCustomCreateXUnitWorkItemsWithTestExclusion -> SDKCustomCreateTestWorkItemsWithTestExclusion (class, file, and UsingTask declaration kept in sync) - SDKCustomXUnitProject item -> SDKCustomTestProject; XUnitWorkItemTimeout -> TestWorkItemTimeout and related targets/properties/task parameters - Updated xunit comment references in AssemblyScheduler.cs and AwaitableProcess.cs Fixes #55094 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR renames the SDK’s Helix “custom work item” MSBuild infrastructure from xUnit-specific identifiers to runner-neutral Test*/SDKCustomTest* naming, reflecting the post-migration reality where the pipeline handles multiple runner styles (including MTP/TRX).
Changes:
- Renames Helix scheduling item/property/target names in
test/UnitTests.projand updates the imported runner targets path. - Renames the compiled MSBuild task
SDKCustomCreate*WorkItemsWithTestExclusionand updates theUsingTask/parameter names accordingly. - Renames/updates runner/publish target files and tweaks wording in a couple of comments to be runner-neutral.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests.proj | Updates Helix scheduling item/property names (SDKCustomTestProject, TestWorkItemTimeout) and imports the renamed runner targets. |
| test/test-runner/TestRunner.targets | Renames properties/targets/tasks to SDKCustomTest* and updates the work item creation task invocation. |
| test/test-runner/TestPublish.targets | Runner-neutral publish helper targets (PublishWithOutput, MTP/TRX detection query targets). |
| test/Microsoft.DotNet.HotReload.Test.Utilities/AwaitableProcess.cs | Updates comment reference to the renamed work-item timeout property. |
| test/HelixTasks/SDKCustomCreateTestWorkItemsWithTestExclusion.cs | Renames the MSBuild task class and its public MSBuild parameters/outputs to Test*. |
| test/HelixTasks/AssemblyScheduler.cs | Updates comments to runner-neutral wording (with a couple of doc/comment fixes suggested). |
Copilot's findings
Comments suppressed due to low confidence (1)
test/test-runner/TestRunner.targets:12
SDKCustomTestArgumentsis given a default value here, but the task invocation later passesTestArguments="$(SDKCustomTestArgument)"(singular). CurrentlySDKCustomTestArgumentis not defined anywhere in the repo, so this default likely never takes effect and the property name may be a typo/leftover.
- Files reviewed: 5/6 changed files
- Comments generated: 3
- Reword AssemblyScheduler XML doc to be runner-neutral (drop xUnit 'Fact' reference) and fix 'inherrited' typo - Fix 'inheritting' typo and tighten grammar in the zero-method class comment - Correct UnitTests.proj breadcrumb to the repo-relative 'test/UnitTests.proj' path Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
MichaelSimons
left a comment
There was a problem hiding this comment.
I personally think SDKCustomCreateTestWorkItemsWithTestExclusion is unwieldy but that is an existing problem. CreateHelixTestWorkItems would be better IMO.
Merging as-is as PR is green and I'll do a follow-up. |
Fixes #55094
What
Renames the runner-agnostic Helix work-item pipeline from xUnit-specific names to runner-neutral ones. After the xUnit → MSTest/MTP migration, this infrastructure is no longer xUnit-specific (it already detects MTP projects via
IsMTPProject/GetIsMTPProjectand handles TRX reporting), so the names were misleading. This is a naming/clarity cleanup only — no functional change intended.Changes
test/xunit-runner/test/test-runner/XUnitRunner.targets/XUnitPublish.targetsTestRunner.targets/TestPublish.targetsSDKCustomCreateXUnitWorkItemsWithTestExclusion(class + file +UsingTask)SDKCustomCreateTestWorkItemsWithTestExclusionSDKCustomXUnitProjectitem,XUnitWorkItemTimeout,SDKCustomXUnit*props, targetsRestore/Build/CreateSDKCustomXUnit*, task paramsXUnitProjects/XUnitArguments/XUnitWorkItemsSDKCustomTest*/TestWorkItemTimeout/Test*equivalentsAssemblyScheduler.cs,AwaitableProcess.csFile/folder renames use
git mvto preserve history. The compiledUsingTask TaskName=…declaration inTestRunner.targetswas kept in sync with the renamed task class.Left intentionally untouched:
Microsoft.DotNet.SdkCustomHelix.Sdknamespace (already runner-neutral).eng/common/**(Arcade-vendored — out of scope per the issue).$(SDKCustomTestArgument)singular-vs-plural mismatch (was$(SDKCustomXUnitArgument)), to avoid any behavior change.Does the custom infra actually differ from Arcade's Helix SDK?
The issue asked to confirm this. Compared against Arcade
main(src/Microsoft.DotNet.Helix/Sdk/): yes, it differs substantially — the fork is justified, Arcade has not caught up.CreateXUnitWorkItemsWithTestExclusion.csandAssemblyScheduler.csdo not exist in Arcade — they were never contributed upstream.AssemblyScheduler, noBaseMethodLimit/MethodLimitMultiplierpartitioning, no per-shard--filter.MTPProjectitem type +CreateMTPWorkItemsthat emitsdotnet exec --roll-forward Major --runtimeconfig … --depsfile …and deliberately omits--report-trx/--diagnostic/--ignore-exit-code 8. The SDK unifies MTP + VSTest under one item type and emits those itself, conditioned on build-time detection.dotnet test+--blame-hang); Arcade uses legacyxunit.console.dll.codesign+entitlements, xUnit v3 apphostchmod +x, RID-qualified publish + CI pre-restore,AdditionalPayloadDir,DOTNET_SDK_TEST_EXECUTION_DIRECTORY/…MSBUILDSDKRESOLVER_FOLDER,.netfxidentity suffix.Validation
HelixTasks.csprojbuilds clean (0 warnings, 0 errors).UsingTaskname must match the class name exactly, and is fully validated only by a green Helix run. Requesting infra-owner review.Risk
Low-to-medium. A mistake would manifest silently as tests not being scheduled on Helix; needs a full green Helix run to confirm.