Declare selftest fixture tiers so packaged fixtures stop going amber - #1155
Open
Alexandre Zollinger Chohfi (azchohfi) wants to merge 1 commit into
Open
Declare selftest fixture tiers so packaged fixtures stop going amber#1155Alexandre Zollinger Chohfi (azchohfi) wants to merge 1 commit into
Alexandre Zollinger Chohfi (azchohfi) wants to merge 1 commit into
Conversation
`Packaged_*` fixtures shared one corpus with the unpackaged tier and self-skipped there, so every unpackaged run left three permanent entries in `SkippedFixtures_AreReported`'s amber inventory — a channel whose value depends on being rare — growing by one line per packaged fixture added. Those fixtures can never assert unpackaged: the gate keys off the entry assembly, so the condition is structural, and a skip is the wrong shape for it. A skip is a per-run observation; this is a fixed property of the fixture. Declare the requirement instead. `SelfTestFixtureRegistry.TierRequirements` maps the three fixtures to `SelfTestTier.Packaged`, and both `--self-test` and `--list-fixtures` are fed from `FixturesForCurrentTier`, so a fixture the host cannot run is not selected, emits no TAP, and gets no test case. The source corpus stays shared and identical; only the runtime selection differs, declared as data in one place. `SkippedFixtures_AreReported` needed no edit — with the fixtures unselected its amber clears on its own — and the `# SKIP` taxonomy, `Harness.Skip` and every existing skip test are untouched. The runtime gate stays and is not redundant. Selection cannot observe identity (it keys off which binary is running), so `RequirePackagedTier` remains what catches the *packaged* host launched without identity, which `IdentityDependentFixtures_Actually_Asserted` turns into a red. Fixing something by removal is the risk here: "no amber" is what you get whether the filter works or whether somebody deleted the packaged corpus. So the host names what it excluded, after `# Total failures:` and with prefixes distinct from the skip trailers, and the two shims assert opposite things — the unpackaged one that the list is non-empty and that none of those names reached discovery or the run, the packaged one that it is *zero*, which is what catches the tier probe mis-evaluating there and silently dropping the only fixtures that tier exists to run. A missing trailer is deliberately never read as a count of zero, or a host that stopped reporting would satisfy the packaged assertion by silence. Verified by mutation: emptying `TierRequirements` reproduces the issue's reported output verbatim (all three `Packaged_*` back in the amber inventory) and reddens `NotApplicableFixtures_AreExcludedFromThisTier`. The packaged tier passes under real MSIX identity with zero exclusions reported. Fixes #1154 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 885f2d1f-9615-477f-8a11-3014730614eb
Alexandre Zollinger Chohfi (azchohfi)
requested a review
from Chris Anderson (codemonkeychris)
as a code owner
August 29, 2026 05:33
Contributor
📦 Build metricsArtifact sizes for Packages (compressed .nupkg)
Assemblies in Microsoft.UI.Reactor
Assemblies in Microsoft.UI.Reactor.Advanced
Assemblies in Microsoft.UI.Reactor.Devtools
No size change beyond the noise floor. ✅ ✅ smaller / |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| $"truncated between the two trailer lines, which would make every check below reason " + | ||
| $"about a partial list.\nNamed: {string.Join(", ", report.Names)}"); | ||
|
|
||
| Assert.IsTrue(report.Count.Value > 0, |
Contributor
🧪 Merged coverageCoverage for
No coverage change beyond the noise floor. ✅ ✅ higher / |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1154.
The problem
Packaged_*fixtures share one corpus with the unpackaged tier and self-skip there, so every unpackaged run left three permanent entries inSkippedFixtures_AreReported's amber inventory — a channel whose value depends on being rare — growing by one line per packaged fixture added.Those fixtures can never assert unpackaged: the gate keys off the entry assembly, so the condition is structural. A skip is a per-run observation; this is a fixed property of the fixture, and restating it every run was the wrong shape.
The approach
Rather than the issue's recommended option B (a machine-readable category on each
# SKIPdirective), applicability is declared on the fixture, once:Both
--self-testand--list-fixturesare fed fromSelfTestFixtureRegistry.FixturesForCurrentTier, so a fixture this host cannot run is not selected, emits no TAP, and gets no test case.This is not the issue's rejected option D. The source corpus stays shared and identical; only the runtime selection differs, declared as data in one place. The visible cost is that
--list-fixturesbecomes tier-dependent — which is exactly the fact being modelled.What it doesn't touch:
SkippedFixtures_AreReportedneeded no edit — with the fixtures unselected its amber clears on its own. The# SKIPtaxonomy,Harness.Skip,TryParseSkipDirective,FixtureStatusand every existing skip test are unchanged.The runtime gate stays, and is not redundant. Selection cannot observe identity — it keys off which binary is running.
RequirePackagedTierremains what catches the packaged host launched without identity, whichIdentityDependentFixtures_Actually_Assertedturns into a red.Keeping an absence honest
Fixing something by removal is the risk here: "no amber" is what you get whether the filter works or whether somebody deleted the packaged corpus. So the host names what it excluded, after
# Total failures:and with prefixes distinct from the skip trailers:The two shims then assert opposite things:
SelfTestBatch.NotApplicableFixtures_AreExcludedFromThisTierPackagedSelfTestBatch.EveryFixture_IsApplicableToThePackagedTierA missing trailer is deliberately never read as a count of zero — that would let a host which stopped reporting satisfy the packaged assertion by silence.
Verification
Mutation-checked, not just green. Emptying
TierRequirementsreproduces the issue's reported output verbatim — all threePackaged_*back in the amber inventory — and reddensNotApplicableFixtures_AreExcludedFromThisTierwith cause (a). So the declaration is demonstrably what removes the amber, and the new test demonstrably reddens when it goes away.dotnet test tests/Reactor.SelfTests—SkippedFixtures_AreReportedno longer lists anyPackaged_*; 3 fewerFixturecases; 9 newTierApplicabilityTestspass.dotnet test tests/Reactor.PackagedTestsunder real MSIX identity — 7/7, zero exclusions reported; 47/47 headless.One unrelated pre-existing failure on my machine:
Issue487_ScrollViewOffsetRestoredAfterRunMutation. CI is green at the base commit (fceaf52), and it reproduces under--filter Issue487, where this change is provably inert — the filtered fixture set is byte-identical either way. Machine-specific (non-interactive desktop), not this PR.Docs
TESTING.md§2 and §3,AGENTS.md, and thePackagedIdentityFixtures/RequirePackagedTierdoc comments now say a packaged fixture needs two declarations and explain why neither replaces the other.probe-aot-skips.ps1drops#lines for the same reason the two shim parsers do.Known trade-off
A fixture wrongly declared packaged-only silently leaves the unpackaged run. It still runs in the packaged CI job, so coverage moves rather than disappears, and the trailer names it on every run. The reverse mistake — a new packaged fixture nobody declared — still self-skips and still goes amber, which is the "you forgot to declare it" signal.