Migrate xUnit v2 tests to xUnit v3 on Microsoft Testing Platform

[Mikael Weaver (mikaelweave)]

Migrate the test suite from xunit v2 to xunit.v3 (3.2.2)

Summary

This moves the test suite from xunit v2 to native xunit.v3 3.2.2 and rewrites the custom
test framework in src/Microsoft.Health.Extensions.Xunit from scratch. The governing requirement
is that net complexity goes down: the rewrite deletes custom infrastructure that xunit.v3 now
provides natively (assembly fixtures, retry) rather than porting it. The testing process is
functionally identical — same FHIR versions (STU3/R4/R4B/R5), same data stores (CosmosDb/SqlServer),
same formats (Json/Xml), same CI legs selecting the same tests. The bulk of the diff is mechanical
find/replace; the framework is the only part that needs careful review.

Context: a prior attempt at this migration was rejected as too large to review (293 files,
+12,738/−968) — it grew the custom framework it should have shrunk, from 1,421 to 3,461 lines.
This PR is a from-scratch redo built off origin/main, optimised for a mostly-mechanical,
complexity-reducing diff.

Framing: this is NOT a test-runner migration

main already runs on Microsoft Testing Platform (MTP) — via the YTest.MTP.XUnit2 v2→MTP
shim (version 1.0.3; global.json sets "test": { "runner": "Microsoft.Testing.Platform" }). This
PR swaps that shim for xunit.v3, which speaks MTP directly. The runner does not change; only the
framework underneath it does. Reviewers expecting a runner cutover will not find one.

One package swap is part of this and is named here so it does not look unexplained: all 28 test
projects move from the xunit.v3 meta-package to xunit.v3.mtp-v2, with
UseMicrosoftTestingPlatformRunner=true. The CI e2e/export legs pass MTP options
(--filter-query, --retry-failed-tests, --report-trx) directly to the test executable;
stock xunit.v3 defaults that executable to the native console runner, which rejects those options
(exit 3). main only worked because the now-deleted YTest.MTP.XUnit2 shim made the executable an
MTP host. xunit.v3.mtp-v2 resolves to MTP 2.0.2, matching the repo's existing Retry/TRX
extension pins, whereas the plain xunit.v3 meta hard-binds MTP 1.9.1 and throws
MissingMethodException at runtime. This is a host-package swap under the same runner, not a runner
migration.

How to review this

The diff falls into four buckets. Only the first needs real attention.

1. Genuinely novel code — the ~1,045-line framework rewrite. THIS IS THE ONLY PART THAT NEEDS
   CAREFUL REVIEW. It lives entirely in src/Microsoft.Health.Extensions.Xunit (13 .cs files):

   • CustomXunitTestFrameworkDiscoverer.cs (417) — the most important file: flag expansion,
     per-dimension replacement, variant construction, the guarded v2-insert display name, trait merge,
     and the fail-loud discovery-fault path.
   • CustomXunitTestFrameworkExecutor.cs (37) plus the runner cascade
     (CustomXunitTestAssemblyRunner, …AssemblyRunnerContext, CustomXunitTestCollectionRunner,
     CustomXunitTestClassRunner) — framework registration and per-variant fixture-argument
     injection. Only CustomXunitTestClassRunner (86) carries real logic; the rest is pass-through
     v3 forces you to declare (see Complexity).
   • FixtureArgumentSetTestClass.cs / FixtureArgumentSetTestMethod.cs / FlagCodec.cs — the
     per-variant class/method types and the shared flag codec, including the IXunitSerializable
     re-implementation (see Risks).
   • FixtureArgumentSetsAttribute.cs, SingleFlag.cs — the fixture-argument-set attribute and the
     flag helper.
     Read these files closely; the design and its five reflection points are described under Risks
     below.

2. Pure mechanical find/replace — safe to skim. These are pattern-verifiable in aggregate:

   • 438 SkippableFact sites rewritten to v3 built-ins (table below).
   • 44 IAsyncLifetime declaration edits across 23 files / 22 types (Task→ValueTask).
     All 15 call sites are plain await; none needs .AsTask().
   • All [RetryFact]/[RetryTheory] call sites converted to plain [Fact]/[Theory] (the
     tests are otherwise unchanged — not rewritten to polling loops): 26 sites across 10 files
     (14 [RetryFact] + 12 [RetryTheory]).
   • [assembly: TestFramework] and using Xunit.Abstractions; housekeeping.

3. Deletions — see the deletion list.

4. Build / CI config — Directory.Packages.props, Directory.Build.props, per-project
   .csproj package swaps, and the CI filter changes (below).

SkippableFact rewrite — 438 sites → 4 mechanical mappings (no shim)

v2 form	count	v3 replacement
[SkippableFact]	199	[Fact]
[SkippableTheory]	46	[Theory]
Skip.If(cond, reason)	161	Assert.SkipWhen(cond, reason)
Skip.IfNot(cond, reason)	32	Assert.SkipUnless(cond, reason)

The 32 reason-less Skip.IfNot sites all take one shared neutral constant rather than
hand-written per-site messages, keeping the change verifiable by pattern. Assert.SkipWhen/
SkipUnless require a reason argument, so any missed site is a compile error — a desirable loud
failure.

Deletions (and why they are safe)

• Custom retry implementation. Deletes the 5 retry files (RetryFactAttribute,
  RetryFactDiscoverer, RetryTestCase, RetryTheoryAttribute, RetryTheoryDiscoverer, 535
  lines) plus IClassFixtureExtensions.cs (141 lines, a retry helper with zero other references) —
  676 lines total. All [RetryFact]/[RetryTheory] call sites become plain [Fact]/[Theory].
  There are 26 such call sites across 10 files (14 [RetryFact] + 12 [RetryTheory]; heaviest
  is ConditionalDeleteTests at 11 — 6 Fact + 5 Theory), and the same grep on the migrated head
  returns 0 (exit 1) — the removal is total. A reviewer's own git grep shows 30 hits across
  11 files; the extra 4 are string literals in DiagnosticMessage($"[RetryFact] …") inside
  RetryTestCase.cs — the retry framework's own source, which this PR deletes. So 26 is the true
  call-site count and 30 is the grep artefact.
  Safe because Microsoft Testing Platform's --retry-failed-tests already covers this at the
  platform level and is already configured on every CI leg on main. Deleting custom retry
  introduces no new masking policy. The tests themselves are not rewritten.
• The Xunit.SkippableFact package dependency. Removed from the consuming .csproj files;
  the 438 sites move to the v3 built-ins above. Across the 28 consuming projects the package
  references consolidate 4 → 1 at most and 3 → 1 at least: xunit +
  xunit.runner.visualstudio + Microsoft.NET.Test.Sdk (+ Xunit.SkippableFact where used)
  collapse into the single xunit.v3.mtp-v2 metapackage — a project sheds 4 references if it used
  SkippableFact, 3 if it didn't. This is the −118 bucket in the decomposition above (29 .csproj
  changed for −123 total; the framework's own .csproj accounts for −5, leaving −118 across the 28
  consumers). Three honest caveats so the 4→1 headline doesn't inflate: xunit.assert →
  xunit.v3.assert and xunit.extensibility.core → xunit.v3.extensibility.core are 1:1 renames,
  not consolidation, and the Newtonsoft.Json removal is incidental, unrelated to the
  migration (7 distinct packages removed, 3 added).
• The YTest.MTP.XUnit2 shim. No longer needed — xunit.v3 speaks MTP natively.
• Assembly fixtures move to native Xunit.AssemblyFixtureAttribute. The 9
  [assembly: AssemblyFixture] declarations across 8 files / 2 fixture types are retargeted to the
  built-in attribute, so no custom assembly-fixture code is carried forward.

Risks and honest disclosures

None of these are buried; read them before approving. Each collapsed item's headline states the
risk — expand for the full analysis.

Five private-reflection points into xunit.v3 internals, pinned to 3.2.2 — each carries a loud type-load guard

The delegate-then-patch model writes exactly five private fields of sealed-getter xunit.v3
types. There is no supported substitute for any of them on 3.2.2. Each is resolved once via a
static readonly FieldInfo and carries a loud type-load guard that names the field, its
declaring type, why it is written, and the 3.2.2 version pin — so a future xunit upgrade that
renames a field fails at type-load with an actionable message instead of silently mis-seeding.

1. XunitTestCase.testCaseDisplayName — per-variant display name.
2. XunitTestClass.uniqueID — per-variant test-class identity (load-bearing; see below).
3. XunitTestMethod.traits — additive DataStore/Format traits on the method.
4. FixtureMappingManager.fixtureCache — seed the chosen (DataStore, Format) for the fixture ctor.
5. FixtureMappingManager.parentMappingManager — reach the parent scope to place the seed.

Fields 2 and 4 are load-bearing: without a distinct per-variant uniqueID, all variants share one
class run and one class-fixture instance, so only one (DataStore, Format) is ever seeded — the
other variants run against the wrong store while displaying the right name and passing green.

v3 silently swallows a discovery exception (fewer tests, still exit 0) — the reason the fail-loud fault path exists

This is the single most important thing to understand about the framework. If a throw escapes
test discovery (FindTestsForType), v3 does not fail the run: the class simply vanishes, the
summary reports fewer tests, and the process still exits 0. A malformed test in a fixtured class
would remove real coverage invisibly. To defend against that, the framework contains a deliberate
fault path that catches a per-method discovery failure and emits a loud, trait-tagged
failing ExecutionErrorTestCase. Do not read this as defensive polish — it is the reason a fault
path exists at all. (Its measured line cost is broken out under Complexity accounting.)

The fault case must survive trait filtering, or the defence is a no-op under the E2E leg. This
was found and fixed (head 66e78c9):

• Before the fix, under a positive filter /[(DataStore=CosmosDb)] the fault cases were
  simply absent — Test run summary: Passed!, total: 6, failed: 0, exit 0. A broken
  discovery run reported success. That is the defect.
• After the fix, the same positive filter gives Failed!, total: 19, failed: 11, exit 2,
  selecting e.g. MethodThrowWithClassAttrTests(CosmosDb, Xml).Bad3a. The fault case now carries the
  union of the raw class- and method-level flags as DataStore/Format traits, so it survives
  positive trait filtering.

Why "positive" is load-bearing: the integration legs exclude a store rather than selecting one,
and an exclusion keeps tests whose DataStore trait is absent as well as different — so those legs
would have caught the fault case regardless and never exposed the bug. On the E2E legs the
DataStore selector is a positive trait equality, so an untraited fault case is dropped by it
regardless of the category clause's polarity — the category default is itself negative
(Category!=ExportLongRunning), but it is ANDed with the DataStore clause and so cannot rescue an
untraited case. Because DataStore is positive on every E2E leg, the /[(DataStore=CosmosDb)]
demonstration above is representative of all of them, not one arbitrary leg; a fix demonstrated only
under a negative filter would have proven nothing.

Named residual: if an attribute's constructor itself throws and the class carries no
class-level attribute, the variant flags are genuinely unknowable, so the error case is emitted
untraited — loud when unfiltered and under negative legs, but still invisible under a positive
filter. This was accepted deliberately rather than fabricating traits: it is strictly better than
v2, where that same input threw out of discovery and the entire class vanished silently, so it
cannot regress against v2.

Cross-process serialization regression — found and fixed; parity with v2 (VSTest bridge 27→1 green before, 29→29 after)

The custom per-variant types were not serializable, so any runner that transports test cases by
serialization (the VSTest bridge / Visual Studio Test Explorer) silently dropped every
variant-expanded test and reported green — the VSTest path discovered 27 tests and executed 1,
exit 0. The FHIR CI runs in-process MTP and was never affected, but this is exactly the
silent-coverage-hole class this migration must not ship.

Framing: this is parity with v2, not new scope. xunit v2 already implemented
TestClassWithFixtureArguments : IXunitSerializable; restoring serializability matches what v2 did.
Implementation note: XunitTestClass.Serialize/Deserialize are sealed (virtual final) in
3.2.2 and cannot be overridden. The fix instead re-declares IXunitSerializable on the subclass
with explicit implementations that chain base.Serialize/base.Deserialize (a sealed method is
still callable via base.) and add the variant flags. This looks odd — converting it to override
will not compile. Cost: +70 lines, no new reflection point. Verified: round-trip OK=28 BAD=0;
VSTest bridge discovery 29 → execution 29.

MTP-native Test Explorer path not exercised (residual, same-shape risk)

The serialization fix is verified through the VSTest bridge. The MTP-native Test Explorer path
(Visual Studio's direct MTP integration) requires Visual Studio and was not exercised in this
work. It is the same shape of cross-process test-case transport as the VSTest bridge, so the same
serialization contract should cover it — but that is unverified. Flagging it as a known residual
rather than a closed item.

Two v3 analyzers (xUnit1031 / xUnit1051) suppressed — test ruleset only, production untouched; disclosed deferral

xUnit1031 (no blocking task operations in tests) and xUnit1051 (prefer
TestContext.Current.CancellationToken) are set to None in the test ruleset only — the
production ruleset is untouched, with zero leakage verified. Both are new in v3 and fire as
errors across hundreds of existing test bodies; clearing them is a behavioural change to test
code and is out of scope for a mechanical migration. Recommended follow-up: address xUnit1031
specifically — it flags a real latent deadlock risk (~171 candidate sites). xUnit1051 is
stylistic and lower priority.

Two executor subsystems deliberately dropped (assertion env-var copy ~80, parallelism semaphore ~66) — formatting/throughput only, never coverage

Two blocks from the old executor were left out on purpose — do not re-add them speculatively:

• Assertion-formatting env-var copying (~80 lines). Consequence: assertion-failure messages
  truncate at the default depth/length rather than a custom one. Affects message formatting only,
  never which tests run or pass/fail.
• Parallelism semaphore (~66 lines). Consequence: MaxParallelThreads is unenforced, so
  collections can all start at once. A throughput/resource difference, never a correctness or
  coverage one.

Re-add either only if a team actually depends on the corresponding option.

Complexity accounting (honest)

Headline, stated plainly because a prior attempt was rejected for growing exactly this code:
the framework directory nets 1,421 → 1,045 (−376), but all of that reduction is deleting the
retry subsystem. For equal functionality the fixture-expansion machinery grew — 745 → 1,045,
+300 (+40.3%) raw, or +183 (+28.6%) once MIT headers and /// docs are stripped from both
sides. Repo-wide the diff is 144 files, +1,467 / −1,967, net −500, but that −500 is retry
deletion plus package consolidation — not the test code shrinking (test .cs moved only −28
across 68 files). Full decomposition below.

Full complexity accounting — repo-wide −500 decomposition, framework 745 → 1,045 (+300) breakdown, discoverer +66 raw, and zero recoverable slack

The framework directory shrank, but all of that reduction is retry deletion; for equal
functionality the fixture-expansion machinery grew. Stating that ourselves, with the breakdown, is
the whole point of this section — a reviewer who checks will find the growth in about two minutes,
and this PR exists because a previous attempt was rejected for growing exactly this code.

Repo-wide this is a substantial net reduction — 144 files, +1,467 / −1,967, net −500 lines — but
the honest story is where the −500 comes from, not the aggregate. It deletes the retry subsystem
(676 lines), the Xunit.SkippableFact package and its 438 call sites, the YTest.MTP.XUnit2 shim,
IClassFixtureExtensions, and 5 dead package pins. The auditor decomposed the diff by path bucket;
it reconciles exactly (residual 0):

Bucket	net
Framework directory (src/Microsoft.Health.Extensions.Xunit/)	−381
Project-file package consolidation (28 .csproj, 4 packages → 1)	−118
Test .cs in the test/ directory (SkippableFact + IAsyncLifetime, 68 files)	−28
Everything else (test .cs under src — 18 files, chiefly SkipReasons.cs — +15, tools +8, misc +4, CI +3, Packages.props −3)	+27
Net	−500

−381 −118 −28 +27 = −500. Read that table before trusting the headline. The intuitive reading
of "net −500" is "the tests got simpler" — that is false, and this body should not permit it.
Test .cs in the test/ directory moved only −28 across 68 files, essentially line-neutral: SkippableFact→Fact
is a one-for-one token swap and the IAsyncLifetime edits are signature changes, so those deletions
land in tokens, not lines. The reduction comes from deleting the retry subsystem (inside the
framework bucket) and consolidating package references (the −118 .csproj bucket — four packages
collapsing to one xunit.v3.mtp-v2 across 28 projects, 4× the test-code contribution) — not from
the test code shrinking.

(The framework directory contributes −381 to the diff: its .cs blobs shrink −376 — the
1,421 → 1,045 comparison below — and the directory's own .csproj a further −5. The −376 is a
blob-state figure and must never be subtracted from a diff total; everything outside the framework
directory nets −119.)

The framework directory itself:

Scope	Lines
v2 framework total (origin/main, 15 .cs files)	1,421
— retry subsystem removed (6 files)	676
v2 non-retry baseline	745
v3 framework total (13 .cs files)	1,045
Rejected prior port (context only)	3,461

Framework directory: 1,421 → 1,045 (−376) (a blob-state .cs comparison). Do not read that as
the story. That −376 is retry deletion (−676) net of the fixture machinery growing the other way:
v3 ships no retry, the 676-line retry subsystem is gone, and for equal functionality the
non-retry baseline against v3 is
745 → 1,045, +300 (+40.3%) raw, or +183 (+28.6%) once MIT headers and /// docs are
stripped from both sides. We state that increase first and plainly: this PR exists because an
earlier attempt was rejected for growing exactly this code, and a reviewer who finds the +300
unaided will distrust everything else here. Where the +300 goes (each column sums exactly to its
total; every line is counted, blanks and braces included):

Category	v2 non-retry	v3	Δ
XML /// doc	70	171	+101
// comments (incl. MIT header)	48	105	+57
blank	123	112	−11
braces — file-split scaffolding (all files ex. discoverer)	76	130	+54
braces — discoverer fault-path nesting	107	127	+20
using	44	54	+10
substantive code	277	346	+69
total	745	1,045	+300

This table is measured — every cell confirmed by direct count, with per-file bucket sums
asserted against the raw totals; no cell was corrected from the earlier provisional pass. The braces
row is split deliberately: +54 is genuine one-type-per-file boilerplate (9 non-retry files became
13, each carrying namespace/class/method scaffolding), but +20 is real fault-path nesting inside
the discoverer (braces 107→127) — the brace-shadow of the new EmitFaultCase / ApplyFlagTraits /
TryGetRawFlags / CollectRawFlags bodies and their try/catch/foreach blocks. Calling all +74
"boilerplate" would understate substantive growth by 20.

So ~53% of the raw growth is XML /// docs plus // comments (the MIT header and inline notes),
~21% is one-type-per-file scaffolding (braces +54, using +10) the impl brief required, and the
rest is substantive — +69 code lines plus the +20 fault-path braces, not boilerplate. The fairest
single comparison — stripping MIT headers and /// docs from both sides — is the +183 (+28.6%)
already quoted above.

Net growth ≠ review surface: one file dominates, and roughly nine you cannot skip

A reviewer who sees "+300 net" will assume one file grew and the rest is noise. That is false, and
the Executor is the counter-example that proves it: CustomXunitTestFrameworkExecutor.cs shrank
251 → 37 (−214) while adding reviewer work — its logic moved into 7 new files (436 lines). Net
growth and review surface are different quantities; allocate attention by the second, not the first.
The 13 .cs files at head split into four review buckets:

• 1 file dominates the diff — CustomXunitTestFrameworkDiscoverer.cs, 144 → 417 (+273 of the
  +300 net). This is the fail-loud discovery path dissected above — the one block that needs a real,
  slow read.
• 9 files you cannot skip — the gutted CustomXunitTestFrameworkExecutor.cs (251 → 37) and the
  rewritten CustomXunitTestFramework.cs (49 → 29), plus the 7 new files the Executor's logic moved
  into: AssemblyRunner (32), AssemblyRunnerContext (37), ClassRunner (86), CollectionRunner
  (30), FixtureArgumentSetTestClass (80), FixtureArgumentSetTestMethod (113), FlagCodec (58).
  (The "4-file runner cascade = 185" cited above is AssemblyRunner+AssemblyRunnerContext+
  ClassRunner+CollectionRunner; the other three — 80 + 113 + 58 = 251 — are the fixture/codec
  machinery counted in the +300 table — 185 + 251 = 436, same files, two groupings.)
• 1 minor append — FixtureArgumentSetsAttribute.cs (+19, 0 deletions): glance only.
• 2 unchanged — NotTest.cs, SingleFlag.cs: skip entirely.

The 9 deletions are not one bucket — they split 6 / 3, and the halves need different reviews:

bucket	files (lines)	what to check
6 retry deletions → confirm-gone	RetryTestCase (316), IClassFixtureExtensions (141), RetryTheoryDiscoverer (91), RetryFactDiscoverer (54), RetryFactAttribute (37), RetryTheoryAttribute (37)	nothing replaces these — the feature is gone, so the only question is "is it really gone?"
3 replaced deletions → verify-the-replacement	TestClassWithFixtureArguments (91) → FixtureArgumentSetTestClass.cs; TestClassWithFixtureArgumentsTypeInfo (76) → v3 native metadata; AssemblyFixtureAttribute (27) → native Xunit.AssemblyFixtureAttribute	behaviour moved — read the replacement; do not just confirm absence

Do not flatten that 6/3 into one list of nine: confirming a retry file is gone is a two-second check,
but confirming TestClassWithFixtureArguments's serialization behaviour survived into
FixtureArgumentSetTestClass is the cross-process parity fix disclosed under Risks — the single
deletion that most rewards a careful read. (Census reconciles: 15 − 9 + 7 = 13; main column 1,421,
head column 1,045.)

Two points that pull the other way and should not be omitted:

• The execution machinery shrank. v2's monolithic executor was 251 lines; v3's executor (37)
  plus the 4-file runner cascade (185) totals 222 — a net −29 for the same job. The cascade
  replaces the executor rather than adding to it, and 99 of its 185 lines are pure pass-through
  that v3 requires you to declare in order to inject a custom class runner. Presenting "+185
  architecture tax" in isolation would be misleading.
• None of the growth is recoverable slack. The auditor read the entire fault-path diff at
  66e78c9f: no dead code, no orphaned helpers (it verified GetVariantClass / ApplyVariantTraits
  are still called on the success path), and the fault path was actually consolidated — two
  duplicated error-case blocks became one EmitFaultCase, a DRY reduction. The only ever-recoverable
  candidate was Fix E (below), permanently closed on the fault path's merit, so the honest figure is
  zero cleanly recoverable lines — we looked, and there is nothing left to cut. The +300 is
  convention overhead + parity + genuine fault-tolerance, with nothing to trim.

On Fix E specifically: a refactor to collapse the TryGetMethods / TryComputeVariants wrappers
was proposed and deliberately rejected. Those wrappers are the guards that stop an exception
escaping FindTestsForType; removing them to recover the ~10–15 lines would reintroduce the silent
class-drop the fault path exists to prevent. In a PR whose thesis is "complexity goes down," we
still declined that reduction because the fault path wins — recorded here as a decision, not an
omission, to pre-empt the obvious reviewer question. With Fix E closed, the cleanly recoverable
total is zero.

The single largest block of genuinely new code is the discoverer's fail-loud discovery path (the
discoverer went 144 → 417 versus v2). Its growth since c498c301 is measured and deliberate —
+66 raw, decomposing cleanly as +45 comment-stripped, +21 inline //, +0 ///
(45 + 21 + 0 = 66, a closure worth keeping visible). That +45 comment-stripped is itself
+19 code, +20 braces from new guarded blocks, and +6 blank lines (19 + 20 + 6 = 45); the braces
and blanks are the scaffolding of splitting fault handling into more guarded blocks, so the genuinely
new logic is the +19. Of the +66, +61 came from three silent-failure fault-path fixes (at
head 66e78c9) and the remaining +5 from a later comment-only Fix C residual — inline hazard
notes, no code, confirmed at the framework level where stripping all comments leaves the total at
769 lines both before and after that edit, delta zero.

The discoverer's /// XML-doc line count is 11 both before and after — delta zero — which is what
makes the growth provably inline // and not XML docs. Over the full range the inline // grew
+21 net (27 added, 6 removed), so a reviewer running git diff c498c301 cb7c6dff7 | grep '^+.*//' sees 27 — the gross, not the +21 net; that the grep returns the gross rather than the
net is exactly what we measured. Of that, the fault-path pass alone contributed +16 net
(22 added, 6 removed), which classified by content is 14 reflection-hazard /
fault-path-invariant lines + 2 algorithm-semantics lines, with 0 throwaway (no section markers,
restatements, TODOs, or formatting breaks). That is the correct home for them: a hazard note like
"do not re-enter these reflected types on the fault path" belongs as a // comment at the hazard
site, not as an XML doc on a public member.
Those 61 lines were not incidental: the maintainer ordered three confirmed silent-failure defects
fixed after each was reproduced by fault injection — a deliberate trade of 61 lines to stop a broken
discovery run exiting 0 green, in the same spirit as the Fix E rejection above.

The fault path exists for one reason, the strongest-verified fact in this
migration: xUnit v3 silently swallows an exception thrown out of FindTestsForType — the test
class simply vanishes, fewer tests run, and the process still exits 0 (a green build with missing
coverage). This was proven by execution twice independently — once during design, and again during
review, when renaming a single reflection lookup made an entire fixtured class disappear at exit 0.
Without that context these lines read as gold-plating; with it, they are the justification for the
largest new block in the PR. (See the discovery-swallow disclosure under Risks.)

For scale: the rejected earlier attempt's framework was 3,461 lines; this is 1,045 — about 3.3×
smaller. That comparison is fair and worth making explicitly, since the reviewers of this PR saw
the earlier one.

Own the obvious objection before a reviewer raises it: for this repo's healthy suite, the fault
path is dead code. ReportFault / EmitFaultCase / ApplyFlagTraits / TryGetRawFlags are
reachable only from the catch around ExpandMethod, which never fires here (0 genuine fault
markers) — so ~20 of the +300 like-for-like lines are code that never runs on a green build. That is
deliberate, not oversight. The trap it guards is xUnit v3 silently swallowing a throw out of
FindTestsForType: a class that breaks during discovery simply disappears and the run still exits
0. Code that never fires is the desired steady state for fault handling — the failure it prevents
is precisely the one that leaves no trace until the day it matters, and a reviewer who confirms the
path is unreached is confirming the suite is currently healthy, not that the guard is waste. It is
also not purely additive: introducing it consolidated two duplicated error-case blocks into a
single EmitFaultCase.

Behaviour preserved

• Test display names keep the exact v2 form Namespace.Class(SqlServer, Json).Method, produced
  by an insert of the (DataStore, Format) suffix after the class name (not an append). This is
  byte-for-byte identical to v2 specifically to preserve Azure DevOps test history. A guarded
  append fallback exists only for a hypothetical future custom DisplayName.
• All FHIR versions (STU3/R4/R4B/R5), both data stores (CosmosDb/SqlServer), and both formats
  (Json/Xml) still run.
• The 22 Format.All sites still expand both Json and Xml within one process. A method-level flag
  replaces the class-level flag for that same dimension (per-dimension replacement), leaving the
  other dimension as declared — so a method marked SqlServer on a CosmosDb class yields
  (SqlServer, Json), a combination the class never declared. This is not an intersection (which
  would make that method vanish) and not a widening; it matches v2 exactly.
• Each CI leg still selects the same set of tests.

CI filter changes

main's VSTest-style substring filters become native xunit.v3 trait queries. This is a grammar
change, not a string swap: native xunit.v3 rejects the legacy --filter option, --filter-query
predicates cannot be mixed with simple filters, and multiple --filter-query values are OR'd (so a
compound DataStore & Category must be one query). Changes span four of the five
build/jobs/*.yml files — run-cosmos-tests.yml, run-sql-tests.yml, e2e-tests.yml,
run-export-tests.yml; build.yml needs no change (its unit legs carry no data-store filter,
and those assemblies contain zero fixtured classes).

CI filter mechanism detail — per-leg before/after, dynamic e2e query, 17-leg selection-neutrality (missing 0 / added 0), and pr-pipeline vs ci-pipeline sql (2105 / 2192)

Leg (polarity)	Before	After
run-cosmos-tests.yml integration (negative)	--filter "FullyQualifiedName!~SqlServer"	--filter-not-trait "DataStore=SqlServer" --filter-not-class "*SqlServer*" †
run-sql-tests.yml integration (negative)	--filter "FullyQualifiedName!~CosmosDb"	--filter-not-trait "DataStore=CosmosDb" --filter-not-class "*CosmosDb*" †
e2e-tests.yml (positive, compound)	--filter "FullyQualifiedName~${appServiceType}&${categoryFilter}"	--filter-query "/[(DataStore=${appServiceType})&(${categoryFilter}…)]" (built dynamically — see below)
run-export-tests.yml (positive, compound)	--filter '…~CosmosDb&Category=ExportLongRunning' / ~SqlServer&…	--filter-query "/[(DataStore=CosmosDb)&(Category=ExportLongRunning)]" / SqlServer

† Simple filters, not --filter-query. This asymmetry is deliberate and required — see
Integration-leg exclusion filters — a regression found late, and fixed below. The first-cut
--filter-query "/[(DataStore!=SqlServer)]" port shipped a bug; these two legs had to move off
--filter-query entirely.

The e2e leg's query is built dynamically in build/jobs/e2e-tests.yml:

$categoryPredicates = "${{ parameters.categoryFilter }}".Split('&') | ForEach-Object { "($_)" }
$query = "/[(DataStore=${{ parameters.appServiceType }})&$($categoryPredicates -join '&')]"
$args = @('--filter-query', $query, '--retry-failed-tests', '3', '--report-trx')

Each category clause is split on &, wrapped in parentheses, re-joined with &, and ANDed with the
DataStore selector — so whatever polarity $categoryFilter carries is preserved verbatim. The
behavioural change is the selection mechanism: v2 matched the datastore by display-name substring
(FullyQualifiedName~${appServiceType}), v3 by DataStore trait equality — same intent,
different mechanism, and the single most consequential change to the CI surface.

✅ Validated by enumeration: the shipped e2e/export filters select the same tests on v2 and
v3. The arbiter rebuilt both sides fresh (v2 origin/main@18e884cd5b, v3 cb7c6dff7) and ran
the actual shipped filter strings — not self-composed predicates — through
--list-tests --filter-query, diffing the results as name-level multisets (the same "same
tests, not the same number of tests" gate used for discovery). Every positive leg is identical, missing 0 / added 0, with no leg yielding exit
8 — and the sweep was repeated on all three FHIR versions CI ships (R4 all 7 legs, Stu3 all
7, R5 the 3 sql legs that exist = 17 legs):

leg	R4 (v2/v3)	Stu3 (v2/v3)	R5 (v2/v3)
cosmos-main	1481 / 1481	1289 / 1289	n/a
cosmos-reindex	52 / 52	52 / 52	n/a
sql-main	2105 / 2105	1918 / 1918	1891 / 1891
sql-reindex	52 / 52	52 / 52	52 / 52
sql-bulkupdate	35 / 35	35 / 35	35 / 35
export-cosmos	3 / 3	3 / 3	n/a
export-sql	5 / 5	5 / 5	n/a

Every cell is missing 0 / added 0, exit 0. n/a = no such stage in the pipelines (there is no
R5 Cosmos E2E stage and no R5 export stage), not an unmeasured or empty leg — publishing a figure
there would manufacture coverage CI never runs. The per-version main-leg totals genuinely differ —
cosmos-main 1481 (R4) vs 1289 (Stu3); sql-main 2105 / 1918 / 1891 — which is itself the proof that
three distinct binaries were enumerated, not one counted thrice; the small legs (reindex 52,
bulkupdate 35, export 3 / 5) repeat across versions only because those categories carry no
version-specific tests and just the filter varies, so their equality is consistent rather than a
copy-paste artefact.

So the FQN-substring→trait and VSTest-Category→MTP-Category conversions are selection-neutral
on every positive leg — on every FHIR version CI runs. Honest scope of the claim: this is validated by offline enumeration, not
by a green pipeline. This PR is opened from a fork (mikaelweave/fhir-server), so it does not receive
pipeline secrets and the e2e/export/integration stages cannot run on it — and the legs whose
filter strings this change rewrites are precisely the legs that cannot execute here. Every predecessor
PR in this effort ran from a branch on upstream microsoft/fhir-server and did get those legs. If a
maintainer wants a live run before merge rather than a membership proof, re-opening this PR from an
upstream branch is sufficient — the commits are unchanged. Absent that, the evidence stands as: the
filters select identical membership on all 17 shipped positive legs and both fixed integration
legs, and the Windows unit leg is green at 14,887 tests / 0 failures; the gap is a live e2e/export
run, not the correctness of the selection or the unit suite. The pipeline that runs on
this PR is pr-pipeline.yml, which passes no category overrides and therefore uses the
template defaults (IndexAndReindex / BulkUpdate); pr-pipeline.yml and ci-pipeline.yml have
zero diff in this PR — only the filter mechanism changed, never the category values.

The two E2E sql figures — 2105 and 2192 — are both correct; they belong to different pipelines.
The seven leg families above are the pr-pipeline.yml legs that run on this PR (the 2105 / 2192 contrast here is the R4 column): it passes no category
override and so takes the template defaults, which split reindex and bulk-update into their own legs
(sql-main 2105 + sql-reindex 52 + sql-bulkupdate 35). After merge, ci-pipeline.yml
overrides with mainCategoryFilter and folds those back into a single leg, so the post-merge main
gate enumerates cosmos 1481 / sql 2192, where 2192 − 2105 = 87 = IndexAndReindex 52 + BulkUpdate 35 (disjoint), and everything still reconciles to 1,536 + 2,197 + 28 = 3,761. Neither value is
"the" E2E sql count — 2105 is pr-pipeline, 2192 is ci-pipeline — and stating either unlabelled
is what makes the two tables look contradictory. This split is pre-existing, not introduced by the
migration: the category values are byte-identical v2→v3, and pr-pipeline.yml/ci-pipeline.yml
have zero diff here. (IndexAndReindex and ReindexOperation are not a typo and neither is a no-op:
each selects the same 104 tests — 52 cosmos + 52 sql — confirmed by name-level diff.)

Polarity is load-bearing. The E2E DataStore selector is a positive trait equality; an untraited
fault case is dropped by it regardless of the category clause's polarity. This holds because
(DataStore=<store>) is ANDed with the category clause, so a negative category default
(Category!=ExportLongRunning, per build/jobs/e2e-tests.yml) cannot rescue an untraited case.
Because DataStore is positive on every E2E leg, the Fix B / Finding 2 demonstration under
/[(DataStore=CosmosDb)] is representative of every E2E leg, not one arbitrary leg — a fault
emitted untraited is invisible on all of them.

Zero-match is kept as a defect signal (MTP exit code 8) — no --ignore-exit-code 8 is added.

Dev tools were ported too. tools/ABTestRunner/Invoke-ABTest.ps1 was first proven broken against
native xunit.v3 (real output: exit 5, stderr Unknown option '--filter'), then converted to
--filter-query "/[(Category!=Export)&(Category!=ExportDataValidation)&(Category!=ExportLongRunning)&(Category!=Import)&(DataStore=SqlServer)]",
with every predicate keeping its original polarity. After the fix the option is accepted, the
query parses, and > 0 tests are selected. tools/MultiInstanceRunner/README.md was likewise
corrected to --filter-query node+trait syntax, alongside a stale -f net9.0 → net10.0 fix.
There is deliberately no clean AFTER exit code for these: a pass/fail verdict needs a live FHIR
server, which this work did not stand up. What is proven is that they clear the exact barrier that
produced the --filter exit 5 — not that a full run goes green.

Integration-leg exclusion filters — a regression found late, and fixed

The two integration legs are the one place a mechanical filter port went wrong, and it is worth
a reviewer's full attention because of how it was found. Both legs replaced a v2 name exclusion
with a v3 trait predicate:

• run-cosmos-tests.yml: --filter "FullyQualifiedName!~SqlServer" → (first cut)
  --filter-query "/[(DataStore!=SqlServer)]"
• run-sql-tests.yml: --filter "FullyQualifiedName!~CosmosDb" → (first cut)
  --filter-query "/[(DataStore!=CosmosDb)]"

What broke. A trait != means "differs or is absent". Store-named classes that carry no
DataStore trait therefore survive the exclusion and get double-scheduled onto the opposite store's
leg. Measured: cosmos 291 → 334 (+43), sql 439 → 446 (+7).

Why it matters — and why it is not coverage loss. Nothing is dropped (missing 0 on both legs);
the defect is extra tests. But the extras are backend-dependent: each is an IClassFixture<…> whose
fixture provisions a live store in InitializeAsync before any test body runs, and an IClassFixture
constructor failure errors every test in the class rather than skipping it. So the Cosmos leg
would attempt 43 SQL-backed tests with no SQL provisioned — both integration legs would have gone
red on main.

Why CI did not catch it — the honest disclosure. There is no integration leg in this PR's check
list at all. The integration legs run only after merge, so this regression could not have surfaced
on the PR; it would have broken main. That is exactly why it was caught by offline name-level
enumeration rather than by a red check here, and it is the single most reviewer-useful fact in this
section.

Why the fix is not "just correct the query". A query-filter fix is impossible on runner 3.2.2,
proven by execution — any explicit path segment collapses the trait predicate to zero matches, and
multiple --filter-query args OR rather than AND, so an exclusion trait cannot be ANDed with a
name exclusion inside --filter-query:

/[(DataStore!=SqlServer)]                      exit 0, found 334   ← the bug
/*/*/*/*[(DataStore!=SqlServer)]               exit 8, found 0
/*/*/!*SqlServer*/*[(DataStore!=SqlServer)]    exit 8, found 0
/*/*/!*SqlServer*   (name negation alone)      exit 0, found 596

The fix. The two negative legs drop --filter-query for simple filters, ANDing a trait
exclusion with a class-name exclusion:

• cosmos: --filter-not-trait "DataStore=SqlServer" --filter-not-class "*SqlServer*" → exit 0,
  found 291
• sql: --filter-not-trait "DataStore=CosmosDb" --filter-not-class "*CosmosDb*" → exit 0,
  found 439

Both clauses are load-bearing: --filter-not-class alone finds 596, --filter-not-trait alone finds
334 / 446; only the AND of the two reproduces v2's exact membership. Verified against the v2
baselines (v2 origin/main, v3 this PR) at name level — missing 0 / added 0 on both legs, so the
fixed legs run the same tests v2 ran, not merely the same count. (These are the shipped-filter CI
legs; distinct from the unfiltered R4 Integration assembly A/B of 707 / 707 under Verification, which
tests a different thing — that the discoverer edit moved no test.)

The asymmetry is deliberate — do not "harmonise" it. The seven positive E2E/export leg definitions stay on
--filter-query; only these two negative integration legs use simple filters. That looks untidy, but
simple filters are the only mechanism on 3.2.2 that ANDs an exclusion trait with an exclusion name.
Converting these two legs back to --filter-query for consistency silently reintroduces the +43 / +7
double-scheduling regression above.

Verification

Result up front — every claim below is measured, cross-model, and expanded in the collapsed detail:

• The same tests run. Name-level multiset diff (not counts): missing 0 / added 0 on every
  enumeration; 87 classes stable (none added, none vanished); display names byte-identical, so
  Azure DevOps test history survives.
• Executed parity spot-check: Microsoft.Health.Fhir.SqlServer.UnitTests 1,053 → 1,053,
  failed: 0.
• Merge gate — all three shipped integration assemblies (R4, Stu3, R5): the two fixed legs
  reproduce their v2 baselines exactly on every version that ships them — cosmos 291 / 291 (R4, Stu3;
  no R5-cosmos leg exists), sql 439 / 439 (R4, Stu3, R5), missing 0 / added 0 — the six binaries
  proved distinct by SHA256, so the identical totals are not one assembly counted thrice.
• Provenance: discovery-neutrality was measured by a separate claude-opus-4.8 arbiter against
  freshly built binaries; fault-path findings were raised by a different-provider gpt-5.6-sol
  reviewer and confirmed by fault injection — not self-verified by the implementer.
• Measurement SHA vs head: all code measurements in this body were taken at cb7c6dff7. The only
  commit since — 8e02cdfb9 — changes two YAML lines and no .cs file (+4/−2), so every line
  count, enumeration and parity figure here holds unchanged at head.

Full verification detail — name-level diff tables, the counting rule, risky-construct coverage, per-assembly reconciliation, and cross-model provenance

The same tests run — verified by name-level diff, not by pass/fail

The central risk of this migration was never "do the tests pass" — a framework rewrite can lose
whole classes and still report green. It is "does the same set of tests still run". This was
measured, not asserted: an independent arbiter built both sides from source with the same
toolchain (c498c301 pre-fix, 66e78c9f post-fix), enumerated every leg with --list-tests, and
diffed the discovered test names — not merely their counts.

The gate is written at name level, because counts alone can lie. A class losing four tests while
another gains four leaves the total unchanged and reads as a perfect pass; only a name-level multiset
diff catches an equal-sized swap. What was actually measured is the stronger statement — no test
name moved — so this section says "the same tests," never "the same number of tests."

Across all seven enumerations of the R4 E2E assembly — unfiltered plus each bare DataStore
and export predicate — v2 and v3 agree exactly, and every enumeration's runner-reported
found N test(s) matched its canonical stripped count on both sides. What this table proves: the
discoverer edit (pre-fix c498c301 vs post-fix 66e78c9f) moved no test within the R4 E2E assembly
under bare trait predicates. What it does not prove: anything about the shipped CI filter
strings — those are validated separately by the positive-leg enumeration above. The rows below are
therefore labelled by assembly and bare predicate, not by CI-leg name (the integration-* /
e2e-* names belong to CI legs and are reserved for them):

R4 E2E enumeration	v2 (main)	v3 (this PR)	name diff
unfiltered	3,761	3,761	missing 0 / added 0
DataStore!=SqlServer	1,564	1,564	missing 0 / added 0
DataStore!=CosmosDb	2,225	2,225	missing 0 / added 0
DataStore=CosmosDb	1,536	1,536	missing 0 / added 0
DataStore=SqlServer	2,197	2,197	missing 0 / added 0
export (DataStore=CosmosDb)	3	3	missing 0 / added 0
export (DataStore=SqlServer)	5	5	missing 0 / added 0

On top of the per-name diff, a whole-class check: 87 classes on both sides — none appeared, none
vanished, and none changed its per-class test count. (The R4 Integration assembly was separately
A/B'd name-for-name at 707 / 707, also identical.) Display names were byte-identical —
Namespace.Class(DataStore, Format).Method — so no normalisation was needed, which additionally
means Azure DevOps test-history keys still match and historical trend data survives the
migration.

Counting rule — stated because getting it wrong has already manufactured two false counts on this
PR. For every discovery figure: trim each --list-tests line before matching, drop the chrome
(banner, Test discovery summary, the duration: footer) and blanks, and treat the runner's own
found N test(s) line as the figure of record. The rule is load-bearing, not pedantry: a v2-era
banner filter silently stopped matching when the v3 banner text changed (producing the dead
1565 / 2226 / 1537 / 4 / 6, each exactly +1), and a two-space-indented duration: footer once
survived a ^duration: anchor. A discovery number published without its counting rule is how the
next wrong number is born — that is this PR's own history, not a hypothetical.

Coverage of the risky construct: of 256 total attribute sites, 146 are method-level across 28
files (the case where per-dimension replacement can produce a class-undeclared combination); 25 of
those 28 files were covered directly, each with a zero diff — including AuditTests (29/29) and
BundleTransactionTests (46/46), the two classes whose class- and method-level attributes conflict
most sharply.

Per-assembly executed count, separately: Microsoft.Health.Fhir.SqlServer.UnitTests runs
1,053 → 1,053 (exact) — v3 reports total: 1053, failed: 0, skipped: 0, zero delta from the v2
baseline.

The seven counts reconcile against the R4 E2E composition — 1,536 (CosmosDb, …) +
2,197 (SqlServer, …) + 28 non-variant = 3,761. The negative-trait legs (!=) include all 28
non-variant tests, because != means differs or absent; the positive-trait legs (=) exclude
them. That asymmetry is why the DataStore=CosmosDb enumeration is 1,536 but the
DataStore!=SqlServer enumeration is 1,564 — the difference is exactly those 28 tests — and
1,536 + 2,197 + 28 = 3,761 closes. A reviewer who spots the mismatch
without this note would assume tests went missing.

This discovery-neutrality was measured by a separate claude-opus-4.8 arbiter against freshly built
binaries on both sides — not by the implementer who wrote the framework — and the fault-path
findings were likewise raised by a different-provider gpt-5.6-sol reviewer and confirmed by fault
injection on claude-opus-4.8 before any fix was written (see below). Self-verified migration claims
are worth less, so the provenance is stated explicitly.

How the fault-path findings were validated (cross-model)

The three fault-path findings above were not accepted on plausibility. They were raised by a reviewer
running gpt-5.6-sol — deliberately a different provider from the claude-opus-4.8 implementer —
and each was then confirmed by fault injection by a separate claude-opus-4.8 verifier that
reproduced it in situ before any fix was written. A fourth finding from the same reviewer was
cleared by an execution A/B and is deliberately not in this PR; a review that only ever confirms
is not a review.

Framework behaviour (from the proven spike and the ported build)

• Acceptance matrix (29 tests): full run failed:0 skipped:4; trait-filter counts
  29 / 2 / 2 / 2 / 15 / 14 (traits merge, not replace).
• Fixture injection: per-variant fixtures receive the correct enum values (tests assert the
  injected (DataStore, Format) internally; Failed:0 proves it).
• Fault path (fixed at head 66e78c9): a discovery failure is converted into a trait-tagged
  failing ExecutionErrorTestCase. Under a positive filter /[(DataStore=CosmosDb)] the fault
  cases now select and fail — total: 19, failed: 11, exit 2 — where before the fix the same
  filter reported total: 6, failed: 0, exit 0 (green over a broken discovery run). Residual:
  a throwing attribute constructor on a class with no class-level attribute is emitted untraited
  and stays invisible under positive filters (see Risks).
• Serialization round-trip: OK=28 BAD=0.
• VSTest bridge (out-of-process): discovery 29 → execution 29 (was 27 → 1 → exit 0 before the
  fix).
• Merge gate: for every CI leg, --list-tests under the new filter must select > 0 tests.
  The two negative integration legs now use simple --filter-not-trait/--filter-not-class filters
  (not --filter-query; see Integration-leg exclusion filters), verified against all three shipped
  integration assemblies (R4, Stu3, R5) to reproduce their v2 !~ baselines at name level — cosmos
  291 / 291 (R4, Stu3), sql 439 / 439 (R4, Stu3, R5), missing 0 / added 0 (binaries proved distinct by
  SHA256). The
  first-cut --filter-query port over-selected (+43 / +7 untraited store-named tests) and is fixed;
  any future divergence from those baselines is a blocking regression.

Build gate: the solution builds in Release with the only remaining errors being the 4 known
pre-existing VerifyExactSdkVersion E2E errors (SDK 10.0.303 vs global.json pin 10.0.302);
global.json is intentionally not edited.

[Mikael Weaver (mikaelweave)]

Two things only the PR owner can action

Everything else on this PR is closed and evidenced in the description. These two are left open
deliberately rather than guessed at.

1. Work-item ID (AB#…) — one-line edit, does not block merge

The Check Metadata workflow asserts the description contains an AB# work-item reference. It is
failing, and it is failing on purpose.

That assertion is a substring check (.github/workflows/validate-prs.yml), so any plausible-looking
number would turn the check green regardless of whether the work item exists. Inventing one would
convert a visible, accurate red into an invisible, false green — so the ID was left for whoever owns
the work item. Check Metadata is not a required status check, so this does not block merge.

Every other assertion in that workflow passes: title is 64 characters (limit 65), and the labels
Enhancement-Test, Build, No-PaaS-breaking-change, No-ADR are set.

2. Fork vs upstream branch — a real decision, and it is cheapest right now

This PR is opened from a fork (mikaelweave/fhir-server). It is the only PR in this effort that
is — #5507 and the superseded stack
(#5756,
#5758,
#5759) all ran from branches on
microsoft/fhir-server.

Cross-fork PRs do not receive pipeline secrets, so the E2E, export and integration stages cannot
run here. That is structural, not a flake — and it lands awkwardly, because those are exactly the
legs whose filter strings this PR rewrites. The description is explicit about this rather than
leaving it to be discovered.

What stands in their place is offline enumeration: the shipped filter strings were run against freshly
built v2 and v3 assemblies and diffed as name-level multisets — 17 legs, missing 0 / added 0,
across all three FHIR versions CI ships. That is genuine evidence that the filters select identical
tests. It is not evidence that the suites ran. The Windows unit leg did run, and is green:
14,887 tests, 0 failures.

If a live run of those legs is wanted before merge, re-opening from a branch on microsoft/fhir-server
is the whole remedy — the commits are identical. It is near-free today (no human review comments
yet) and gets progressively more expensive once review starts, so it is worth deciding early.

[Mikael Weaver (mikaelweave)]

Superseded by #5762.

This PR was opened from a fork (mikaelweave/fhir-server), which meant it received only a partial
pipeline matrix: cross-fork PRs get no service connection, so the e2e, export and integration stages
could not run — and those are precisely the legs whose test-selection filters this change rewrites.

#5762 contains the identical commits (head 8e02cdfb9, 144 files, +1,467 / -1,967) on a branch in
this repository, so it receives the full leg matrix and those stages execute. Please review #5762.