Skip to content

Add the xUnit extensions test harness and test assets (3/3) - #5759

Closed
Mikael Weaver (mikaelweave) wants to merge 1 commit into
mikaelweave-xunit3-core-migrationfrom
mikaelweave-xunit3-test-harness
Closed

Add the xUnit extensions test harness and test assets (3/3)#5759
Mikael Weaver (mikaelweave) wants to merge 1 commit into
mikaelweave-xunit3-core-migrationfrom
mikaelweave-xunit3-test-harness

Conversation

@mikaelweave

Copy link
Copy Markdown
Contributor

Layer 3 of 3 in the stack splitting #5507 (xUnit v2 -> v3 + Microsoft Testing Platform migration). Targets mikaelweave-xunit3-core-migration (L2, #5758), which targets mikaelweave-xunit3-prep-collection-definitions (L1, #5756).

Merge L1, then L2, then this.

What this adds

The test harness for the xUnit extensions themselves, held back from L2 so that the v3 flip could be reviewed on its own.

  • src/Microsoft.Health.Extensions.Xunit.UnitTests/ (48 files) - the harness covering the extensions.
  • src/Microsoft.Health.Extensions.Xunit.TestAssets/ (68 files) - 34 deliberately-misbehaving mini xUnit programs used as fixture data. TestAssetRunner launches one per scenario as a subprocess and asserts on the published TRX plus the process exit code.

Plus the six coupling edits L2 could not carry because the harness did not exist yet: the solution project entry, its four build-configuration lines and its NestedProjects mapping; the R4.slnf/R5.slnf entries; the unit test project manifest going from 19 back to 20; and the InternalsVisibleTo entry in Microsoft.Health.Extensions.Xunit.

Why TestAssets is not in the solution

Deliberate, and please keep it that way. The assets are programs that are supposed to fail. The project sets IsTestProject=false before the repo props import and is absent from the solution and both solution filters, so nothing discovers them or runs their failing tests directly. They are built solely through a ReferenceOutputAssembly="false" ProjectReference from the harness.

Verification

Check Result
Stack reconstructs the reviewed tree exactly git diff HEAD b7513423c5e40ab9dda4e338d66dea41a6c8cfda is empty
Solution build 0 errors attributable to this change
Harness suite 150/150 passed, 0 failed, 0 skipped
Unit test manifest guard 20/20 matched, exit 0
Solution integrity Both .slnf valid JSON, harness present, TestAssets absent
Tree Clean, single commit

The solution build reports 4 errors from the repo's pre-existing VerifyExactSdkVersion target (installed SDK 10.0.303 vs global.json 10.0.302), all in E2E projects and unrelated to this change. global.json was deliberately not touched. Verified around it the same way L2 did, by compiling the four E2E projects individually with -t:Compile -p:TargetFramework=net10.0: all four returned 0 errors, 0 warnings.

Reviewer note

TestAssetRunner uses --filter-query, which is assembly-wide rather than namespace-scoped. Adding further concurrent query-filtered runs will destabilise sibling tests and surface as MTP exit code 8 / "zero tests ran" in tests you did not touch.

Adds Microsoft.Health.Extensions.Xunit.UnitTests, the harness covering the
xUnit extensions themselves, along with Microsoft.Health.Extensions.Xunit.TestAssets,
a set of deliberately-misbehaving mini xUnit programs used as fixture data.
TestAssetRunner launches one asset per scenario as a subprocess and asserts on
the published TRX plus the process exit code.

Restores the coupling edits the harness requires: the solution and R4/R5 solution
filter entries, the unit test project manifest count (19 -> 20), and the
InternalsVisibleTo entry in Microsoft.Health.Extensions.Xunit.

TestAssets is intentionally excluded from the solution and solution filters: it
sets IsTestProject=false before the repo props import so it is never treated as
a test project or auto-discovered.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@mikaelweave

Copy link
Copy Markdown
Contributor Author

Superseded by #5761.

That PR re-does this migration from scratch against main as a single change, and is a net reduction: 144 files, +1,467 / -1,967, net -500 lines. This stack grew the custom test framework; #5761 deletes the retry subsystem (676 lines), the Xunit.SkippableFact package and its 438 call sites, and the YTest.MTP.XUnit2 shim, taking src/Microsoft.Health.Extensions.Xunit/ from 1,421 to 1,045 lines.

Test selection is preserved at name level - every CI leg was enumerated against a v2 control and diffed as a name multiset (missing 0 / added 0), not compared by count.

Closing to keep review attention on #5761. The branch is not deleted, so this can be reopened if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants