microsoft / microsoft/microsoft-ui-reactor

[Bug] Selftest/E2E fixture registries: a fixture registered only in the `Create()`/`Build()` switch is silently never run, and CI cannot detect it

Open
#1,025 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-triage
Dominant language
C#
Stars
646
Forks
54
Avg merge
1d 3h
Merged PRs (30d)
84

Description

### What happened?

`AGENTS.md` documents that fixture registration is **two-place** — `AllFixtures` **and** the `Create()`/`Build()` switch. It doesn't say that the two omission directions have wildly different blast radii. Only one is loud; the other is silent all the way through CI, and **the selftest job's own test-case list is derived from the very place that can be wrong**, so it is structurally incapable of noticing.

| direction | outcome | detected by |
|---|---|---|
| **A** — name in `AllFixtures`, missing from the switch | `Create()` returns `null` → `not ok - fixture not found` + `RecordFailure()` | **the runner, loudly** (`SelfTestRunner.cs:296-302`) |
| **B** — class + switch arm present, name missing from `AllFixtures` | never enumerated, no TAP line, no MSTest case, suite green | **nothing in the repo** |

Direction B is silent at every layer:

- `SelfTestRunner.cs:242` iterates `SelfTestFixtureRegistry.AllFixtures`, so the fixture is never constructed or run.
- `Program.cs:11-18` (`--list-fixtures`) prints `AllFixtures`, so the fixture is not listed.
- `SelfTestBatch.cs:458` obtains its fixture names *from* `--list-fixtures`, and `:385` feeds them to `[DynamicData(nameof(AllFixtures))]` — so the fixture **never becomes an MSTest case**. The CI "Selftests" job reports a clean, complete-looking run.
- A filtered run (`--self-test --filter DataGrid`) is the worst case: the filter simply matches one fewer name and every remaining fixture passes. `# Total failures: 0`.

Both registries have a non-throwing default arm, so neither direction is a compile error:

- `tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs` — `AllFixtures` at `:10`, `Create()` at `:1652`, `_ => null` at `:3231` (file is 3233 lines).
- `tests/Reactor.AppTests.Host/FixtureRegistry.cs` — `AllFixtures` at `:14`, `Build()` at `:188`, `_ => null` at `:349`.

**No test cross-checks the two places.** `grep -r AllFixtures tests/` returns 7 hits, all of them *consumers*; there is no consistency assertion in either direction.

This is currently mitigated only by manual discipline ("run `--list-fixtures` and check your new names appear"), on a 3233-line file being edited concurrently by several in-flight PRs. That mitigation is correct — it targets exactly direction B, which is the only silent one — but it is unenforced, and the failure it prevents leaves no trace when it does happen: a fixture that never runs looks identical to a fixture that passes.

### Steps to reproduce

1. Add a new selftest fixture class under `tests/Reactor.AppTests.Host/SelfTest/Fixtures/`.
2. Add its arm to the `Create()` switch in `SelfTestFixtureRegistry.cs`, but **omit** the name from the `AllFixtures` array (the realistic slip: the two edits are ~1600 lines apart in the same file, and merge resolutions touch them independently).
3. Build the host and run `dotnet run --project tests/Reactor.AppTests.Host -- --self-test --filter `.
4. Observe `# Total failures: 0` and no TAP line for the new fixture. Nothing reports that a registered fixture was skipped.
5. `dotnet test tests/Reactor.SelfTests` likewise produces no MSTest case for it — the case list came from `--list-fixtures`, i.e. from `AllFixtures`.

Reverse the omission (name in `AllFixtures`, no switch arm) and step 3 fails loudly with `not ok … - fixture not found`, which is the contrast that makes direction B easy to under-rate.

### Reactor version / commit

`8d3db0a2` (PR #1010 head); the registries are unchanged from `main` on this branch — the defect is in repo test infrastructure, not in that PR.

### Platform

x64

### .NET SDK version

.NET 10 SDK (per `global.json`)

### Windows version

Windows 11

### Windows App SDK version

As resolved by the repo's `Microsoft.WindowsAppSDK` reference.

### Logs / stack trace

```
# Direction A (loud) — SelfTestRunner.cs:296-302
var fixture = SelfTestFixtureRegistry.Create(fixtureName, harness);
if (fixture is null)
{
Console.WriteLine($"not ok {testIndex} {fixtureName} - fixture not found");
harness.RecordFailure();
crashed = true;
}

# Direction B (silent) — nothing enumerates the switch, so there is no output to show.
```

### Suggested fix

A headless test in `tests/Reactor.Tests` that parses both registry files with Roslyn and asserts the two places agree as **sets**, reporting the symmetric difference in both directions. This is an established idiom in the repo (`ModifierTableIntegrityTests`, `SetEventSubscriptionConsistencyTests`, `DataGridCaptureSiteTests`), `Microsoft.CodeAnalysis.CSharp` is already a `PackageReference` in `Reactor.Tests`, and it is headless-safe — metadata/syntax only, constructing no `Microsoft.UI.Xaml` object. Extract the string literals from the `AllFixtures` array initialiser and the constant patterns from the `Create()`/`Build()` switch arms.

Two anti-vacuity requirements, because **both oracles are set comparisons and an empty-vs-empty comparison passes for free**:

- Assert a non-trivial count on *each* side before comparing (e.g. `>= 100` for the selftest registry), so a mis-targeted or silently-failing extraction fails loudly rather than certifying agreement between two empty sets.
- Prove it by mutation in both directions — remove one name from `AllFixtures` (expect failure naming it), restore, remove one switch arm (expect failure naming it), restore. A guard for a two-place hazard that has only been run in the passing direction has only been shown to be *green*, not to be *able to go red*.

Filed out of the #987 / #1010 work after verifying the asymmetry directly; happy to implement it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs and FixtureRegistry.cs, then review the existing ModifierTableIntegrityTests, SetEventSubscriptionConsistencyTests, and DataGridCaptureSiteTests patterns in Reactor.Tests. The completed test should compare fixture names from each registry, reject empty or undersized extraction, and fail with the missing name when either registry is mutated.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.