Extract shared collection definitions and add unit test manifest guard - #5756
Extract shared collection definitions and add unit test manifest guard#5756Mikael Weaver (mikaelweave) wants to merge 1 commit into
Conversation
…anifest guard Preparatory refactor ahead of the xUnit v2 to v3 migration. Everything here is inert on xUnit v2 and is split out so the migration itself reviews as a single mechanical change. Collection definitions are moved out of the test classes that declared them and onto dedicated, trait-free definition classes: - ModelInfoProviderCollection serialises the SqlServer unit test classes that read or write the process-global ModelInfoProvider. A class fixture only orders the classes that share it, so it cannot stop a third class overwriting the same static; one shared collection can. - CustomQueriesTestsCollection serialises the CustomQueries tests, which share static state. - IndexAndReindexCollection replaces the duplicate [CollectionDefinition] that ReindexTests and CustomSearchParamTests each declared for the same collection name. The definition deliberately carries no traits, because a trait declared on a shared definition would be inherited by every member of the collection and silently dropped by CI legs that filter that category out. Also adds build/jobs/scripts/Assert-UnitTestProjectsDiscovered.ps1 and its manifest. The unit test leg runs whatever '**/*UnitTests/*.csproj' matches, so a project that leaves the glob stops being tested without anything failing. The script checks the glob against the manifest before the tests start. It is not wired into the pipeline yet; the YAML that calls it lands with the migration. Verified inert: Microsoft.Health.Fhir.SqlServer.UnitTests reports 1053 passed / 0 failed both with and without this change, and the R4 E2E project compiles with the new shared file registered in the .projitems. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 407889a1-346a-479d-9340-5d02c0278d8b
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5756 +/- ##
==========================================
- Coverage 78.61% 78.59% -0.02%
==========================================
Files 1019 1019
Lines 37659 37659
Branches 5729 5729
==========================================
- Hits 29606 29599 -7
- Misses 6654 6659 +5
- Partials 1399 1401 +2 🚀 New features to boost your workflow:
|
|
Superseded by #5761. That PR re-does this migration from scratch against 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. |
Description
Preparatory refactor ahead of the xUnit v2 → v3 migration. Everything here is inert on xUnit v2 — it is split out so the migration itself reviews as a single mechanical change.
This is the bottom layer of a 3-PR stack:
Extensions.Xunitrewrite, CPM/props switch, ~109 test sources, 27 csproj, Tests.Common guards, CI pipeline YAMLExtensions.Xunit.UnitTestsharness +.TestAssetsLayer 2 is irreducible:
Directory.Packages.propsdeletes the xUnit v2 package versions globally under Central Package Management,Directory.Build.propsfans the MTP references out to every project matchingContains('Test'), andMicrosoft.Health.Extensions.Xunitbinds a single xUnit major that every test project references. Those three jointly force one atomic flip, which is why this prep layer contains only changes that are already correct on v2. The CI pipeline YAML rides with layer 2 for a related reason:mainalready runs xUnit v2 under Microsoft Testing Platform via theYTest.MTP.XUnit2shim, which accepts VSTest-style--filter, whereas xUnit v3's MTP requires--filter-query. Swapping the shim without flipping the YAML would leave every integration and E2E leg selecting nothing.Collection definitions
Collection definitions are moved out of the test classes that declared them, onto dedicated trait-free definition classes:
ModelInfoProviderCollection— serialises the SqlServer unit test classes that read or write the process-globalModelInfoProvider. A class fixture only orders the classes that share it, so it cannot stop a third class overwriting the same static; one shared collection can.CustomQueriesTestsCollection— serialises the CustomQueries tests, which share static state.IndexAndReindexCollection— replaces the duplicate[CollectionDefinition]thatReindexTestsandCustomSearchParamTestseach declared for the same collection name. The definition deliberately carries no traits: a trait on a shared definition is inherited by every member of the collection, and a CI leg filtering that category out would silently drop members that only joined to be serialised.Manifest guard
Adds
build/jobs/scripts/Assert-UnitTestProjectsDiscovered.ps1and its manifest. The unit test leg runs whatever**/*UnitTests/*.csprojmatches, so a project that leaves the glob — renamed, moved, or its directory suffix quietly changed — stops being tested without anything failing. The script checks the glob against the manifest before the tests start.It is not wired into the pipeline yet; the YAML that calls it lands with the migration. Its automated tests live in the harness project in layer 3, so this script ships here without them.
Related issues
Addresses AB#189958.
Testing
Verified by execution on the unmodified v2 tree:
Microsoft.Health.Fhir.SqlServer.UnitTests— 1053 passed / 0 failed, both with and without this change, confirming the collection extraction is a true no-op (identical totals rule out silently-dropped tests).Assert-UnitTestProjectsDiscovered.ps1— printsMatched 19 unit test project(s) against 19 expected, exit code 0. The manifest reads 19 here and goes to 20 in layer 3, when the harness project exists.Microsoft.Health.Fhir.R4.Tests.E2E— compiles clean (0 warnings, 0 errors) withIndexAndReindexCollection.csexplicitly registered inMicrosoft.Health.Fhir.Shared.Tests.E2E.projitems, so the new shared file genuinely compiles into consumers rather than being silently skipped.Note:
dotnet buildof the E2E projects trips the repo'sVerifyExactSdkVersiontarget locally because the installed SDK is 10.0.303 whileglobal.jsonpins 10.0.302. That is a pre-existing local-environment condition unrelated to this change, soglobal.jsonwas left untouched and the E2E verification was run via theCompiletarget instead.FHIR Team Checklist
Semver Change (docs)
Skip (test-only preparatory refactor, no product code or behavior change)