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.

  Granularity caveat — the substitution is not like-for-like at every site, and here is where it
  isn't. Two different retry mechanisms are being deleted, and only one of them maps cleanly onto
  --retry-failed-tests:

  • [RetryFact]/[RetryTheory] retried the whole test method. --retry-failed-tests 3 (4
    attempts) also retries the whole test method. Same granularity, same unit of work — a clean
    substitution.
  • IClassFixtureExtensions.RetryAsync was an inner, per-operation retry inside a test body,
    and is a genuine granularity change. Its actual behaviour, read off the deleted source rather than
    assumed: MaxNumberOfAttempts = 3 (up to 4 executions), no backoff at all, and it caught
    only SocketException in the connection-reset family — everything else was rethrown on the
    first failure. All three call sites used the single-argument overload, so the optional
    "additional retriable exceptions" set was always null. It therefore never protected an assertion:
    an eventual-consistency miss surfaces as an xUnit EqualException, which it rethrew immediately.

  For that one failure class it did catch, whole-test retry is a coverage superset — it re-runs the
  body, including the socket operation. The honest difference is that it re-runs the whole body
  rather than resuming mid-test; at the three affected sites the test data is keyed on a
  Guid.NewGuid() tag regenerated per invocation, so a whole-test attempt re-seeds cleanly instead of
  colliding with the previous attempt's data. One of the three sites is inside a method already marked
  [Fact(Skip = …)] and never executes at all.

  Removing the helper initially left three dead await ((Func<Task>)(async () => { … }))() no-op
  wrappers where the retry call had been — real dead scaffolding, not a behaviour change. Those are
  inlined in d3e65a87b (−9 lines); git grep for that shape returns 0 at head.

• 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,528 / −2,037, net −509, but that −509 is retry
deletion plus package consolidation — not the test code shrinking (test .cs moved only −37
across 68 files). Full decomposition below.

Full complexity accounting — repo-wide −509 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,528 / −2,037, net −509 lines — but
the honest story is where the −509 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 + retry-residue inline, 68 files)	−37
Everything else (test .cs under src — 18 files, chiefly SkipReasons.cs — +15, tools +8, misc +4, CI +3, Packages.props −3)	+27
Net	−509

−381 −118 −37 +27 = −509. Read that table before trusting the headline. The intuitive reading
of "net −509" is "the tests got simpler" — that is false, and this body should not permit it.
Test .cs in the test/ directory moved only −37 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: the membership
equality tabulated above was established by offline enumeration — and it has since been
confirmed by live CI on this PR. This PR runs from a branch on
upstream microsoft/fhir-server, so it receives the full pipeline matrix and the e2e / export /
integration stages — precisely the legs whose filter strings this change rewrites — do execute
here. They now have: the build completed succeeded — 42 / 42 jobs and 22 / 22 stages, zero
failures, zero skipped stages — and its per-leg executed
counts match two independent v2 control builds, test-for-test and skip-for-skip, on every
comparable leg (table in the next section). Live execution is the primary gate; the enumeration
below remains because it explains why those legs pass and would have pinpointed the exact leg and
filter clause responsible had any of them not. The offline
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 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.

Live CI evidence — this PR's executed counts vs two independent v2 control builds (18 legs, all match)

Everything above this point is offline enumeration: --list-tests run locally under the committed
filter strings. This section is the live gate — what Azure DevOps actually executed.

• Target (this PR, v3): build 51080, head d3e65a87b. Final result succeeded —
  42 / 42 jobs, 22 / 22 stages, 0 failed, 0 skipped.
• Controls (v2, main + an unrelated change): build 51065 (PR Extend middleware with Inbound Request Logs #5755) and build 51056
  (PR Add Query Store performance diagnostics #5723).

Two controls, not one, deliberately: a PR build is main plus that author's change, so a single
baseline cannot distinguish "the migration moved this number" from "this number moves on its own".
Where the two controls disagree with each other, the leg is natively variable and no conclusion
about the migration can be drawn from it.

Reading the numbers: total / skipped, taken from the first Test run summary in each task
log — with --retry-failed-tests a log can contain several summaries, and only the first one is the
full selection (later ones are the retried subset only). Pass/fail is taken from the job result,
never from a summary; every job in every row below ends succeeded on all three builds.

Leg (ADO task)	v2 #51065	v2 #51056	v3 #51080 (this PR)
E2E R4 CosmosDb	1508 / 29	1508 / 29	1508 / 29
E2E R4 SqlServer	2132 / 24	2132 / 24	2132 / 24
E2E R5 SqlServer	1918 / 28	1918 / 28	1918 / 28
E2E Stu3 CosmosDb	1316 / 40	1316 / 40	1316 / 40
E2E Stu3 SqlServer	1945 / 39	1945 / 39	1945 / 39
E2E R4 CosmosDb Reindex	52 / 0	52 / 0	52 / 0
E2E Stu3 CosmosDb Reindex	52 / 0	52 / 0	52 / 0
E2E R4 SqlServer Reindex	52 / 0	52 / 0	52 / 0
E2E Stu3 SqlServer Reindex	52 / 0	52 / 0	52 / 0
E2E R5 SqlServer Reindex	52 / 0	52 / 0	52 / 0
E2E R4 SqlServer BulkUpdate	35 / 0	35 / 0	35 / 0
E2E Stu3 SqlServer BulkUpdate	35 / 2	35 / 2	35 / 2
E2E R5 SqlServer BulkUpdate	35 / 0	35 / 0	35 / 0
Integration Cosmos (R4)	302 / 20	302 / 20	302 / 20
Integration Cosmos (Stu3)	302 / 107	302 / 107	302 / 107
Integration SQL (R4)	449 / 6	451 / 6	449 / 6
Integration SQL (Stu3)	448 / 117	452 / 117	449 / 117
Integration SQL (R5)	449 / 117	452 / 117	449 / 117

Fifteen of eighteen legs are identical across all three builds. That includes every E2E leg on
every FHIR version and both data stores — 8,819 selected cases across the five main E2E legs
alone — plus all five reindex legs and all three bulk-update legs.

The skip column is the load-bearing one. [SkippableFact] / Skip.If → [Fact] /
Assert.SkipWhen is the single largest semantic conversion in this PR (438 sites). If any of those
193 skip decisions had inverted or been dropped, a test would flip between the skipped and executed
columns without changing the total. The skip count is identical to both v2 controls on all eighteen
legs — 29, 24, 28, 40, 39, 0×5, 0, 2, 0, 20, 107, 6, 117, 117 — 529 skips in total, reproduced
exactly. That is live proof that the skip conversion is decision-for-decision faithful, and it is
stronger evidence than the static count-and-polarity audit that preceded it.

The three legs that differ, and why they are not a finding. Integration SQL varies by up to ±3 —
but the two v2 controls disagree with each other on all three of those legs (449/448/449 vs
451/452/452), so this leg is natively variable on v2 and the migration cannot be the cause. In every
case v3 lands inside the v2 range. Note also that the variance is entirely in the succeeded column:
the skip count on those same three legs is invariant at 6 / 117 / 117 across all three builds. This
is also the residue of the 439-vs-438 reconciliation above — the leg simply does not have one
fixed number.

Disclosed, not hidden — reindex is flaky on v2 as well. Some Reindex jobs record failures on
their first attempt and pass on retry. On this PR that is E2E R4 CosmosDb Reindex (4 first-attempt
failures). On the v2 controls it is E2E Stu3 CosmosDb Reindex (5 first-attempt failures on
#51065) and three separate legs on #51056. Every one of these jobs ends succeeded on all three
builds. Pre-existing category flakiness, present on both sides of the migration, not introduced here.

Independent corroboration from coverage. Codecov reports 78.65 % line coverage for this PR,
against 78.70 % (#5755) and 78.78 % (#5723) on the two v2 control builds — a spread of
0.13 pp. Coverage is instrumented from what actually executed, so it is derived independently of
every count in this description. A migration that silently stopped running a slice of the suite would
show up here as a coverage drop; it does not.

Not comparable, and excluded rather than fudged: the export legs and the unit-test legs run under
different task names / conditions across these builds, so they are not tabulated; the unit leg is
reported separately above (14,887 / 0).

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

Build configuration of the integration figures. Every integration count quoted in this section
(291, 439, 334, 446, 596) was enumerated from Debug binaries. CI builds Release
(build/build-variables.yml:5, buildConfiguration: 'Release'), where the sql figures are 438 /
445 and the name-negation probe is 595 — each exactly one lower. The difference is always the
same single test:
SqlDataReaderExtensionsTests.GivenASqlDataReader_WhenReadingFieldsWithIncorrectCorrectNamesAndOrdinals_Throws,
which sits inside #if DEBUG (…Shared.Tests.Integration/Persistence/SqlDataReaderExtensionsTests.cs:101-119).
It is a statically-Skip'd [Fact], and a skipped fact is still discovered, so it counts at list
time under Debug and does not exist at all under Release. It is the only #if DEBUG / #if RELEASE
conditional in the entire test/ tree at either SHA, so no other count here can move with configuration.

Why the cosmos figures behave differently from each other. That test carries DataStore=SqlServer,
so the cosmos leg's trait clause excludes it under either configuration — which is why the shipped
cosmos figure (291) and the trait-only figure (334) are configuration-invariant. But its bare type
name contains SqlDataReader, not SqlServer, so the name clause does not match it: the
name-negation-only probe is the single selection in which it survives, and therefore the one cosmos
figure that moves (596 Debug / 595 Release). Class-name filtering matches the bare or
namespace-qualified type name only — the (SqlServer) variant decoration is not matched, verified
by probe (/*/*/*(SqlServer)* matches 0 tests).

The parity conclusion is configuration-invariant. That guard is identical on the v2 and v3 sides,
so it cancels: the sql leg is 439 / 439 in Debug and 438 / 438 in Release, missing 0 / added
0 either way. The counts below are left at their as-measured Debug values rather than restated,
because the surrounding text quotes runner output verbatim.

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. (Debug
  binaries; the Release equivalent is sql 438 / 438, same missing 0 / added 0 — see the build
  configuration note above.)
• 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. Two
  commits have landed since, and neither disturbs a figure above. 8e02cdfb9 changes two YAML lines
  and no .cs file (+4/−2). d3e65a87b inlines three dead no-op wrappers left behind by the retry
  helper's removal, touching two test .cs files (+68/−77, net −9) — bodies only: an A/B
  enumeration across all four affected legs before and after that commit returned diff 0 on every
  leg, no test name added, removed or renamed. The repo-wide ledger below is stated at head and
  already includes it.

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; Debug binaries — Release is sql 438 / 438, same missing 0 / added 0, see the build
  configuration note under Integration-leg exclusion filters). 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.

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@18e884c). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5762   +/-   ##
=======================================
  Coverage        ?   78.65%           
=======================================
  Files           ?     1020           
  Lines           ?    37763           
  Branches        ?     5749           
=======================================
  Hits            ?    29703           
  Misses          ?     6650           
  Partials        ?     1410           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Mikael Weaver (mikaelweave)]

Status: full pipeline is green — one owner action left

ADO build 51080 completed succeeded: 42 / 42 jobs, 22 / 22 stages, 0 failed, 0 skipped.
GitHub checks: 47 pass / 0 fail / 1 pending.

Every stage this change actually touches has now executed for real — all five E2E legs on both data
stores across R4 / R5 / STU3, all five reindex legs, all three bulk-update legs, and all five
integration legs. Their executed counts match two independent v2 control builds test-for-test and
skip-for-skip; see the new “Live CI evidence” section in the PR description for the 18-leg
table, including the three legs that differ and why the controls prove the migration is not the cause.

---

The one thing I cannot do for you: Check Metadata

This is the only non-passing check, and it is not a test or build failure. The workflow requires a
DevOps work item reference in the PR description:

if (body.toLowerCase().includes('ab#') == false && ...)
  errors += '- FHIR Team: A DevOps workitem is required. Use AB#123 syntax or link to DevOps item.'

Action: add AB#<id> to the PR description with the real work item number. I deliberately did not
invent one — a fabricated ID would link this PR to the wrong work item, and that is worse than a red
check. This is a one-line edit and the check goes green.

(Note: the run against this head recorded startup_failure, so the check currently reads pending
rather than fail. Re-running it after the AB# is added will resolve both.)

---

Why this PR replaced #5761

#5761 was raised from a fork. On this repository a cross-fork PR does not get the secrets needed by
Setup Test Environment, that stage failed, and it cascaded downstream. Both PRs run the same
22-stage pipeline — but on the fork build (51075)
only 7 stages ran, 1 failed and 14 were skipped, so not one test leg ever executed
(26 GitHub checks, 7 of them failing). On this PR, all 22 stages ran and all 22 succeeded (48 checks,
0 failing). Since the entire question this change raises is "do the test legs still select and run the
same tests?", #5761 could not be evaluated on its merits.

This PR runs from a branch on microsoft/fhir-server itself, which is why the matrix above exists.
#5761 is closed and superseded; the head commit is identical apart from the retry-residue cleanup
(d3e65a87b).

---

Suggested reading order for reviewers

The description is long because the claims are quantified, but it is structured for skimming — every
section past the summary is collapsed. If you only read three things:

1. “Live CI evidence” — the 18-leg table above. This is the primary gate.
2. “Net growth ≠ review surface” — which files actually need human eyes (six, not 144).
3. “Integration-leg exclusion filters — a regression found late, and fixed” — the one place the
   mechanical port went wrong, how it was caught, and the proof it is fixed.

Net change is −509 lines across 144 files: this migration deletes custom test infrastructure
rather than adding to it.