microsoft / microsoft/microsoft-ui-reactor
[Flaky test] Two selftest fixtures still throw on a missing precondition, truncating TAP and hiding every later check
- Dominant language
- C#
- Stars
- 646
- Forks
- 54
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 84
Description
### What happened?
A `throw` inside a selftest fixture aborts that fixture and **truncates the TAP stream**, so every check after it is never emitted. The run then reports a single failure with no attribution, which reads downstream as an arbitrary-victim flake rather than as a precondition that did not hold.
#971 fixed this in `TabViewFillContentAreaFixtures.cs` and measured the effect directly: with the guard reverted and the body unrealized, **only 9 of 16 checks were emitted**; with the fix in place, all 16. The failure count is not the interesting part — the *evidence destruction* is.
Two genuine instances remain after that fix. Both verified against source at `d79813be`, not inferred.
### Site 1 — `WrapElementSlotFixtures.cs:43-46`
A near-exact clone of the fixed case, and the cheapest possible fix because the `H.Check` is already there:
```csharp
var tabView = H.FindControl(_ => true);
H.Check("WrapElementSlot_TabViewMounted", tabView is not null);
if (tabView is null)
throw new InvalidOperationException("TabView control was not mounted."); // <- aborts + truncates
```
The check already reports the failure correctly. The `throw` on the next line then destroys the remaining output for no added signal. One-line change:
```csharp
if (tabView is null) return;
```
### Site 2 — `Win2DCanvasFixtures.cs:327-333` (worse, despite one caller)
```csharp
private static WeakReference CaptureControlWeakReference(Harness h)
where TControl : Microsoft.UI.Xaml.DependencyObject
{
var control = h.FindControl(_ => true);
if (control is null)
throw new InvalidOperationException($"Expected {typeof(TControl).Name} in visual tree.");
return new WeakReference(control);
}
```
Worse than site 1 for two reasons, even though it currently has exactly **one** caller (`Win2DCanvasFixtures.cs:42`, `CanvasControl`):
1. **There is no `H.Check` at all**, so the failure is not merely truncating — it is entirely unreported. Nothing in the TAP stream says a precondition failed.
2. **It is a helper**, so the throw propagates into whichever fixture called it, and the message names a **type** (`CanvasControl`) rather than the fixture. A future second caller inherits the hazard silently.
Suggested shape — make the failure reportable at the call site rather than fatal in the helper:
```csharp
private static WeakReference? TryCaptureControlWeakReference(Harness h, string checkName)
where TControl : Microsoft.UI.Xaml.DependencyObject
{
var control = h.FindControl(_ => true);
h.Check(checkName, control is not null);
return control is null ? null : new WeakReference(control);
}
```
### Important: do NOT sweep all `throw new` in this tree
An audit of `tests/Reactor.AppTests.Host/SelfTest/`:
| count | file | assessment |
|---|---|---|
| 35 | `DevtoolsFixtures.cs` | mechanical `Result(resp) ?? throw` — needs its own decision, not this issue |
| 4 | `ReconcilerBigCoverageFixtures.cs` | unaudited |
| 3 | `NativeDockingRoleAwareFixture.cs` | unaudited |
| 2 | `FlexPanelCssBehaviorFixtures.cs` | unaudited |
| 2 | `ErrorBoundaryFixtures.cs` | **almost certainly intentional — throwing is the thing under test** |
| 1 | `WrapElementSlotFixtures.cs` | **defect, site 1 above** |
| 1 | `Win2DCanvasFixtures.cs` | **defect, site 2 above** |
`ErrorBoundaryFixtures.cs` is the reason a blanket sweep would be wrong: a fixture that exercises error handling *must* throw, and converting it would delete the test. The defect is narrower than "a throw in a fixture" — it is specifically:
> **a `throw` used as a precondition guard, where the fixture could instead report `H.Check(name, false)` and `return`.**
Anything whose throw is the stimulus, or is caught by the fixture itself, is out of scope.
### Verification the fix needs
- After converting, confirm the check still **fails** when the precondition genuinely does not hold — a `return` that skips past the `H.Check` would silently drop the check entirely, which is worse than the throw it replaced.
- **Emit the check unconditionally, not only on the failure path.** #971's `TabViewFill_ExplicitMounted` is written this way deliberately: a check that appears *only* when it fails cannot be distinguished from a check that never ran, so the absence of a `not ok` line stops being evidence of success.
### Reactor version / commit
`main` as of 2026-07-31, plus #971 at `d79813be` (which fixes the `TabViewFillContentAreaFixtures.cs` instance and establishes the pattern). Neither file here is touched by #971 — filed rather than folded in, to keep that PR scoped.
### Notes
Sites identified by the PR merge coordinator session's audit; verified against source and the `ErrorBoundaryFixtures` exclusion added here. Related: #971 (fixes the first instance, with the 9-of-16 measurement), #988 (TAP truncation is one of the mechanisms that makes a failure unattributable).
Contributor guide
Research direction
Start with WrapElementSlotFixtures.cs:43-46 and Win2DCanvasFixtures.cs:327-333, then inspect the CanvasControl caller at line 42 and the existing pattern from #971. Verify each precondition check is emitted unconditionally and that the fixture returns without truncating TAP output when the control is absent. Done means both sites report a failed check while preserving later checks, without changing intentional throws such as those in ErrorBoundaryFixtures.cs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100