diff --git a/AGENTS.md b/AGENTS.md index eb972a034..a5187c3a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,7 +144,7 @@ Optionally: a factory method in `src/Reactor/Elements/Dsl.cs`, fluent modifiers |---|---|---| | Algorithm, pure function, hook bookkeeping, D3 math | Unit test (xUnit) | `tests/Reactor.Tests/` | | Element mount/update against real WinUI controls | Selftest fixture | `tests/Reactor.AppTests.Host/SelfTest/Fixtures/` | -| Behaviour that differs under MSIX identity (`ms-appx:`, `Package.Current`, MRT, `PackageRuntime.IsPackaged` branches) | Selftest fixture gated with `PackagedIdentityFixtures.RequirePackagedTier` | same folder; runs for real in `tests/Reactor.PackagedTests` | +| Behaviour that differs under MSIX identity (`ms-appx:`, `Package.Current`, MRT, `PackageRuntime.IsPackaged` branches) | Selftest fixture gated with `PackagedIdentityFixtures.RequirePackagedTier` **and** declared `SelfTestTier.Packaged` | same folder; runs for real in `tests/Reactor.PackagedTests` | | Real user input, UIA properties, cross-process | E2E test (winapp ui) | `tests/Reactor.AppTests/Tests/` | Start with unit tests. Use selftests only when you need a live WinUI control. E2E is the slowest tier. @@ -156,6 +156,13 @@ adds only MSIX properties plus a `Package.appxmanifest`. The whole corpus runs u **Gotcha worth not re-deriving:** the manifest's `uap5:AppExecutionAlias` is load-bearing — launching the alias stub inherits stdout while keeping package identity, which AUMID activation cannot do (it is brokered, so stdout can't be redirected at all). +**Second gotcha:** a packaged fixture needs *two* declarations, not one. The +`RequirePackagedTier` gate decides whether the body asserts; `SelfTestFixtureRegistry.TierRequirements` +decides whether the unpackaged host runs it at all. Skip the second and the fixture self-skips +into the amber skip inventory on every unpackaged run forever (issue #1154). Consequence worth +knowing: **`--list-fixtures` is tier-dependent**, and both hosts print +`# Total not-applicable fixtures:` / `# Not applicable fixture list:` after `# Total failures:` +so the exclusion is an assertable fact rather than a silent absence. ### Console-mutating tests need collection isolation diff --git a/TESTING.md b/TESTING.md index e712f5005..0a210a8c4 100644 --- a/TESTING.md +++ b/TESTING.md @@ -132,7 +132,7 @@ The reliable tell is the module label. A run that started prints the handshake-d | An algorithm, pure function, record equality, hook bookkeeping, D3 math — anything that doesn't need a WinUI window | **Unit test** in `tests/Reactor.Tests/` | | How an element mounts/updates against a real WinUI control, layout math against real Yoga+XAML, reconciler behavior end-to-end, assertions via `VisualTreeHelper` | **Selftest fixture** in `tests/Reactor.AppTests.Host/SelfTest/Fixtures/` (registered in `SelfTestFixtureRegistry`, wrapped by a `[TestMethod]` in `SelfTestBatch`) | | Real user input (clicks, keystrokes, tab navigation), UIA properties as seen by assistive tech, cross-process behavior, XAML Island interop | **E2E test** in `tests/Reactor.AppTests/Tests/` | -| Anything that changes under **MSIX package identity** — `ms-appx:` resolution, `Package.Current`, MRT lookups, `PackageRuntime.IsPackaged` branches | **Selftest fixture** as above, gated with `PackagedIdentityFixtures.RequirePackagedTier` so it self-skips in the unpackaged tier | +| Anything that changes under **MSIX package identity** — `ms-appx:` resolution, `Package.Current`, MRT lookups, `PackageRuntime.IsPackaged` branches | **Selftest fixture** as above, gated with `PackagedIdentityFixtures.RequirePackagedTier` **and** declared `SelfTestTier.Packaged` so the unpackaged tier does not run it | Rule of thumb: start with a unit test. Drop to selftest only when you need a live control. Reach for E2E only when you need cross-process UIA — E2E is the slowest and flakiest tier. @@ -302,12 +302,20 @@ why SKIPPED is not just a politer green. Always put an issue number in the reaso one; the reason string is all a reader of the skip report gets. And if you are reaching for a skip to silence a flake, fix the flake instead — a skip makes the flake invisible rather than absent. +**"This tier structurally cannot run it" is not on that list, and must not be expressed as a skip.** +A skip is a per-run observation; tier applicability is a fixed property of the fixture, so a skip +would restate the same permanent fact on every run and accumulate in the amber inventory — which is +what [#1154](https://github.com/microsoft/microsoft-ui-reactor/issues/1154) was. Declare the tier in +`SelfTestFixtureRegistry.TierRequirements` instead and the fixture is simply not selected; see §3. + For raw-TAP consumers (the AOT job pipes `--self-test` straight to a `.tap` artifact and greps `^not ok `), the Host emits a `# Total skipped fixtures: N` trailer — placed *after* `# Total failures:` so the abort discriminator below is unaffected — followed by `# Skipped fixture list: ` when non-zero. Each fully-skipped fixture also gets its own -`# Fully skipped fixture: - N check(s) skipped, 0 assertions ran` line as it happens. The -three prefixes are deliberately distinct so a grep for one does not match the others. +`# Fully skipped fixture: - N check(s) skipped, 0 assertions ran` line as it happens. +Alongside them, and reporting a different thing, come `# Total not-applicable fixtures: N` and +`# Not applicable fixture list: ` (§3) — fixtures that deliberately did **not** run here. +All five prefixes are deliberately distinct so a grep for one does not match the others. One fixture, `SelfTestVerdict_OnlySkips_PositiveControl`, is **expected** to be Skipped on every run. It asserts nothing on purpose: it is the positive control that proves the SKIPPED verdict @@ -520,20 +528,58 @@ while still inheriting stdout — so the TAP contract and flags from tier 2 are ### Writing a packaged fixture -Fixtures live in the shared corpus (`tests/Reactor.AppTests.Host/SelfTest/Fixtures/`) and gate -themselves: +Fixtures live in the shared corpus (`tests/Reactor.AppTests.Host/SelfTest/Fixtures/`). Two steps, +and both are load-bearing. **Gate** the fixture body: ```csharp if (!PackagedIdentityFixtures.RequirePackagedTier(H, this)) return; ``` -They run for real here and emit a single TAP skip in the unpackaged tier. Register them with the -**`Packaged_`** prefix — that is what the shim's `IdentityDependentFixtures_Actually_Asserted` -guard uses to decide which fixtures must never skip. +and **declare** its tier in `SelfTestFixtureRegistry.TierRequirements`, beside its entry in +`AllFixtures`: + +```csharp +["Packaged_MyNewThing"] = SelfTestTier.Packaged, +``` + +Register it with the **`Packaged_`** prefix — that is what the shim's +`IdentityDependentFixtures_Actually_Asserted` guard uses to decide which fixtures must never skip. + +The two steps do different jobs and neither replaces the other. The **declaration** governs +*selection*: an undeclared fixture is offered to every tier, so the unpackaged host runs it, hits +the gate, and emits a skip that lands in the run's amber skip inventory — a permanent entry +describing a condition that is structural rather than incidental +([#1154](https://github.com/microsoft/microsoft-ui-reactor/issues/1154)). The **gate** governs +whether the body *asserts*: if this tier were ever launched without identity, it skips, and +`IdentityDependentFixtures_Actually_Asserted` turns that into a red. Selection cannot do that job, +because selection cannot observe identity — it keys off the entry assembly, and the unpackaged +binary is a different binary. + +So `--list-fixtures` is **tier-dependent**: the unpackaged host does not list, and does not run, +the fixtures declared `SelfTestTier.Packaged`. Both hosts print what they excluded, after +`# Total failures:`: + +``` +# Total not-applicable fixtures: 3 +# Not applicable fixture list: Packaged_IdentityGuard, Packaged_SettingsStoreRoundTrip, … +``` + +That trailer exists because the fix for #1154 was a *removal*, and success and catastrophe produce +the same observation when you fix something by removing it: "no amber" is what you get whether the +filter works or whether somebody deleted the packaged corpus. Both shims assert on the trailer, +and they demand opposite things — `SelfTestBatch.NotApplicableFixtures_AreExcludedFromThisTier` +requires a non-empty list and that none of those names reached discovery or the run; +`PackagedSelfTestBatch.EveryFixture_IsApplicableToThePackagedTier` requires **zero**. A tier probe +stuck on one answer would otherwise look correct from whichever side agreed with it. For the same +reason a *missing* trailer is never read as a count of zero — that would let a host which stopped +reporting satisfy the packaged assertion by silence. The gate keys off the entry assembly, not `PackageRuntime.IsPackaged`: a fixture that skipped whenever identity was missing would report green if this tier ever ran without it. +If a fixture needs the *absence* of identity, `SelfTestTier.Unpackaged` is the mirror declaration; +nothing uses it yet. + ### Scope and knobs The whole corpus runs under identity (~5 min, on its own CI runner). diff --git a/tests/Reactor.AppTests.Host/Program.cs b/tests/Reactor.AppTests.Host/Program.cs index bfc5dcc62..4a1d2cc1b 100644 --- a/tests/Reactor.AppTests.Host/Program.cs +++ b/tests/Reactor.AppTests.Host/Program.cs @@ -12,8 +12,14 @@ { // Fast path: emit the selftest fixture registry, one name per line, and exit. // Used by Reactor.SelfTests to discover fixtures without launching WinUI. - foreach (var name in SelfTestFixtureRegistry.AllFixtures) + // + // Deliberately the CURRENT TIER's corpus, not the whole registry: a fixture this host + // cannot run must not get a test case that could only ever report "skipped" (issue #1154). + // The two wrappers' list parsers both drop `#` lines, so the trailer below is inert to + // discovery while still naming the exclusions for a human running this by hand. + foreach (var name in SelfTestFixtureRegistry.FixturesForCurrentTier) Console.WriteLine(name); + SelfTestRunner.WriteNotApplicableTrailer(); return; } diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PackagedIdentityFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PackagedIdentityFixtures.cs index 268ba3b48..8dcce54db 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PackagedIdentityFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PackagedIdentityFixtures.cs @@ -10,16 +10,26 @@ namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; /// /// /// These live in the shared fixture corpus rather than in a packaged-only source -/// set, so the two hosts stay a single body of tests and --list-fixtures agrees -/// across both tiers. What differs is the gate below. +/// set, so the two hosts stay a single body of tests. What differs is selection: +/// they are declared SelfTestTier.Packaged in +/// SelfTestFixtureRegistry.TierRequirements, so the unpackaged host neither lists +/// nor runs them — --list-fixtures is deliberately tier-dependent (issue #1154). +/// They previously ran everywhere and self-skipped, which restated a permanent structural +/// fact as a per-run observation and left three entries in the amber skip inventory on +/// every unpackaged run. +/// The gate below is not redundant with that declaration. Selection cannot +/// observe identity — it keys off which binary is running, and the packaged host binary is +/// a different binary. The gate is what catches the packaged host being launched +/// without identity: a broken registration, a stale alias resolving to something +/// else, someone running the .exe out of the build output. There the fixtures skip and +/// PackagedSelfTestBatch.IdentityDependentFixtures_Actually_Asserted turns that into +/// a red, rather than the suite reporting green while measuring nothing. /// Why the gate keys off the entry assembly. A fixture that merely skipped -/// whenever PackageRuntime.IsPackaged was false would be worse than useless: if -/// the packaged tier ever launched the app without identity — a broken registration, a -/// stale alias resolving to something else, someone running the .exe out of the build -/// output — every identity check would quietly skip and the suite would report green -/// while measuring nothing. That is precisely the failure mode this tier exists to -/// remove. Keying off the entry assembly instead makes the requirement structural: the -/// packaged host binary must have identity, and says so by failing. +/// whenever PackageRuntime.IsPackaged was false would be worse than useless: it +/// would treat the missing-identity case as an excuse rather than a fault, which is +/// precisely the failure mode this tier exists to remove. Keying off the entry assembly +/// instead makes the requirement structural: the packaged host binary must have +/// identity, and says so by failing. /// internal static class PackagedIdentityFixtures { @@ -48,10 +58,17 @@ internal static class PackagedIdentityFixtures /// false. /// /// - /// The skip's check name is derived from rather than passed + /// The skip's check name is derived from rather than passed /// in, so callers cannot invent three different spellings for the same concept and the /// name always points at the fixture a reader has to go look at. Call it as - /// RequirePackagedTier(H, this). + /// RequirePackagedTier(H, this). + /// On the normal unpackaged path this is unreachable, because the fixture is + /// declared SelfTestTier.Packaged and never selected there (issue #1154). It stays + /// because it guards a case selection cannot see: the packaged host running without + /// package identity. There the skip is what + /// PackagedSelfTestBatch.IdentityDependentFixtures_Actually_Asserted converts into a + /// failure — so this must keep skipping rather than assert, or that guard would never see + /// the condition it exists to report. /// internal static bool RequirePackagedTier(Harness h, SelfTestFixtureBase fixture) { diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 160bb6fa1..ca01019de 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -1220,8 +1220,8 @@ internal static class SelfTestFixtureRegistry // Spec 036 — Window model live-shell coverage "WindowModel_LifecycleEvents", "WindowModel_WindowIconApplied", - // Packaged (MSIX) tier. These self-skip in the unpackaged host; - // Reactor.PackagedTests runs them with real package identity. + // Packaged (MSIX) tier. Declared SelfTestTier.Packaged in TierRequirements below, so the + // unpackaged host does not run them at all — see issue #1154. "Packaged_IdentityGuard", "Packaged_SettingsStoreRoundTrip", "Packaged_WindowIconFromResource", @@ -1746,6 +1746,99 @@ internal static class SelfTestFixtureRegistry SkipVerdictPositiveControl.FixtureName, ]; + // ════════════════════════════════════════════════════════════════════ + // Tier applicability + // ════════════════════════════════════════════════════════════════════ + + /// + /// Which host a fixture needs. — the overwhelming default — + /// means "runs everywhere"; the other two mean the fixture is structurally unable to + /// assert anywhere else. + /// + /// + /// is unused today and is defined because the concept is + /// symmetric, not speculatively: Reactor branches on PackageRuntime.IsPackaged in both + /// directions (FileSettingsStore is the mirror of PackagedSettingsStore), so the + /// opposite gate is one declaration away. It costs no extra code path — the test is + /// required == Any || required == current. + /// + internal enum SelfTestTier + { + /// Runs in every tier. The default for a fixture with no declaration. + Any, + + /// Needs MSIX package identity; only Reactor.PackagedTests can run it. + Packaged, + + /// Needs the absence of package identity; only the unpackaged host can run it. + Unpackaged, + } + + /// + /// Fixtures that only apply to one tier. Everything absent from this map is + /// . + /// + /// This is the declaration that removes a fixture from the other tier's corpus + /// entirely — it is not run there, emits no TAP, and gets no test case. Before issue #1154 + /// these fixtures ran everywhere and self-skipped, which put three permanent entries in + /// SkippedFixtures_AreReported's amber inventory on every unpackaged run: a channel + /// whose value depends on being rare, describing a condition that is structural rather than + /// incidental. Declaring the requirement says the same thing once, as data. + /// + /// Keep the runtime gate too. A fixture declared here must still call + /// PackagedIdentityFixtures.RequirePackagedTier: this map governs selection, + /// the gate governs whether the body asserts, and the two are deliberately + /// independent. If the packaged host is ever launched without identity the gate skips and + /// PackagedSelfTestBatch.IdentityDependentFixtures_Actually_Asserted turns that into a + /// red — which a selection-only mechanism could not do, because selection cannot observe + /// identity. + /// + private static readonly Dictionary TierRequirements = + new(StringComparer.Ordinal) + { + ["Packaged_IdentityGuard"] = SelfTestTier.Packaged, + ["Packaged_SettingsStoreRoundTrip"] = SelfTestTier.Packaged, + ["Packaged_WindowIconFromResource"] = SelfTestTier.Packaged, + }; + + /// The tier this process is, derived from the same entry-assembly probe the fixtures use. + internal static SelfTestTier CurrentTier => + PackagedIdentityFixtures.IsPackagedTier ? SelfTestTier.Packaged : SelfTestTier.Unpackaged; + + /// The tier requires; when undeclared. + internal static SelfTestTier RequiredTier(string fixture) => + TierRequirements.TryGetValue(fixture, out var tier) ? tier : SelfTestTier.Any; + + private static bool AppliesToCurrentTier(string fixture) + { + var required = RequiredTier(fixture); + return required == SelfTestTier.Any || required == CurrentTier; + } + + /// + /// The corpus this host actually runs. Both --self-test and --list-fixtures use + /// it, so discovery and execution cannot disagree about the set. + /// + /// + /// A property rather than a cached static readonly array on purpose: the cached form + /// would silently depend on being declared textually after and + /// , and moving either — an ordinary-looking edit in a 3000-line + /// registry — would evaluate it against a null array or an empty map and quietly return the + /// wrong corpus. Recomputing costs one scan of ~1500 strings, on the two or three calls a + /// process makes, against a suite measured in minutes. + /// + public static string[] FixturesForCurrentTier => Array.FindAll(AllFixtures, AppliesToCurrentTier); + + /// + /// The corpus this host deliberately does not run, named so the exclusion is a reported + /// fact rather than a silent absence. The Host prints these as a TAP trailer and both wrappers + /// assert on them: the unpackaged one that they really were excluded, the packaged one that + /// this list is empty — because a packaged run that filtered its identity fixtures out + /// would go green having measured nothing. + /// + public static string[] FixturesNotApplicableToCurrentTier => + Array.FindAll(AllFixtures, name => !AppliesToCurrentTier(name)); + public static SelfTestFixtureBase? Create(string name, Harness harness) => name switch { "HarnessGuard_ClickButtonFailsLoudly" => new HarnessGuardFixtures.ClickButtonFailsLoudly(harness), diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestRunner.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestRunner.cs index a9d0b72b9..0b314cb2d 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestRunner.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestRunner.cs @@ -90,6 +90,51 @@ internal static class SelfTestRunner /// internal const string SuiteElapsedMarker = "# Suite elapsed: "; + /// + /// TAP comment for the count of fixtures this tier deliberately did not run: + /// # Total not-applicable fixtures: <n>, followed by + /// when non-zero. + /// + /// + /// Why an absence gets a trailer. Issue #1154 removed three Packaged_* + /// fixtures from the unpackaged corpus rather than letting them self-skip into the amber + /// inventory. But "the fixtures are not here" and "the fixtures were deleted" look identical + /// from outside, and so do "the tier filter works" and "the tier filter silently matched + /// nothing". Naming the excluded set turns the exclusion into an observation both wrappers + /// can assert on — the unpackaged one that it happened, the packaged one that it did + /// not. + /// Emitted after # Total failures:, for the same reason the skip trailer is: + /// that line is the documented "the Host reached the end of its run" discriminator. The + /// prefixes are deliberately distinct from # Total skipped fixtures: / + /// # Skipped fixture list: so a grep for one cannot match the other. Both literals are + /// duplicated in the two wrappers, which cannot reference this assembly + /// (ReferenceOutputAssembly=false) — change one, change all three. + /// + internal const string NotApplicableCountMarker = "# Total not-applicable fixtures: "; + + /// + /// TAP comment listing the excluded fixture names, comma-separated. See + /// . + /// + internal const string NotApplicableListMarker = "# Not applicable fixture list: "; + + /// + /// Reports the fixtures this tier does not run. Shared by --self-test and + /// --list-fixtures so the two can never disagree about the exclusion. + /// + /// + /// Reports the whole tier-excluded set regardless of : applicability is a + /// property of the tier, and a --filter run narrowing this list would make the + /// wrappers' assertions depend on which subset happened to be selected. + /// + internal static void WriteNotApplicableTrailer() + { + var excluded = SelfTestFixtureRegistry.FixturesNotApplicableToCurrentTier; + Console.WriteLine(NotApplicableCountMarker + excluded.Length); + if (excluded.Length > 0) + Console.WriteLine(NotApplicableListMarker + string.Join(", ", excluded)); + } + private static TimeSpan ResolveHangTimeout() { var env = Environment.GetEnvironmentVariable("REACTOR_SELFTEST_HANG_TIMEOUT_SECONDS"); @@ -339,12 +384,21 @@ public static void RunAll() // committed skip is a configuration error, so it aborts the run // via the catch below rather than being reported as a result. // Validated against the full registry, not `fixtures`, so a - // --filter run doesn't flag every unrelated pattern. + // --filter run doesn't flag every unrelated pattern. It stays the + // FULL registry rather than the tier's slice for the same reason: + // a pattern naming a fixture this tier doesn't run is still a valid + // pattern, not a stale one. ValidateDefaultSkipPatterns(allFixtures); + // The tier's corpus, not the registry's. A fixture declared for the other + // tier is structurally unable to assert here, so it is not run at all — + // rather than run and self-skip, which put a permanent amber entry in the + // run's skip inventory on every run (issue #1154). + var tierFixtures = SelfTestFixtureRegistry.FixturesForCurrentTier; + var fixtures = Filter is not null - ? allFixtures.Where(f => f.Contains(Filter, StringComparison.OrdinalIgnoreCase)).ToArray() - : allFixtures; + ? tierFixtures.Where(f => f.Contains(Filter, StringComparison.OrdinalIgnoreCase)).ToArray() + : tierFixtures; harness.SetupTitleBar(fixtures.Length); window.Activate(); await Harness.Render(); // wait for initial layout @@ -550,6 +604,7 @@ public static void RunAll() Console.WriteLine($"# Total skipped fixtures: {skippedFixtures.Count}"); if (skippedFixtures.Count > 0) Console.WriteLine($"# Skipped fixture list: {string.Join(", ", skippedFixtures)}"); + WriteNotApplicableTrailer(); Console.WriteLine(SuiteElapsedMarker + Stopwatch.GetElapsedTime(suiteStart).TotalSeconds .ToString("F1", global::System.Globalization.CultureInfo.InvariantCulture)); diff --git a/tests/Reactor.AppTests.Host/probe-aot-skips.ps1 b/tests/Reactor.AppTests.Host/probe-aot-skips.ps1 index 581558f2e..507b9e2fd 100644 --- a/tests/Reactor.AppTests.Host/probe-aot-skips.ps1 +++ b/tests/Reactor.AppTests.Host/probe-aot-skips.ps1 @@ -40,8 +40,10 @@ $patterns = [regex]::Matches($skipBlockMatch.Groups[1].Value, '"([^"]+)"') | For $exactPatterns = $patterns | Where-Object { -not $_.EndsWith('*') } $wildcardPatterns = $patterns | Where-Object { $_.EndsWith('*') } -# Get full fixture list once. -$allFixtures = & $exe --list-fixtures | Where-Object { $_ -match '\S' } +# Get this tier's fixture list once. TAP comments are dropped: --list-fixtures ends with the +# tier-exclusion trailer (issue #1154), and a '#' line reaching the wildcard expansion below +# would be probed as though it were a fixture name. +$allFixtures = & $exe --list-fixtures | Where-Object { $_ -match '\S' -and $_ -notmatch '^\s*#' } # Expand wildcards against the fixture registry. $fixturesToTest = New-Object System.Collections.Generic.HashSet[string] diff --git a/tests/Reactor.PackagedTests/PackagedHarnessTests.cs b/tests/Reactor.PackagedTests/PackagedHarnessTests.cs index a266990c7..535b7c379 100644 --- a/tests/Reactor.PackagedTests/PackagedHarnessTests.cs +++ b/tests/Reactor.PackagedTests/PackagedHarnessTests.cs @@ -242,6 +242,74 @@ public void Fixture_List_Filter_Matches_Case_Insensitive_Substring() CollectionAssert.AreEqual(new[] { "Packaged_One", "packaged_three" }, names); } + // ── Tier applicability (issue #1154) ─────────────────────────────── + + /// + /// The distinction EveryFixture_IsApplicableToThePackagedTier rests on. That assertion + /// demands a count of zero, so if a missing trailer parsed as zero, a host that stopped + /// reporting altogether would satisfy it by silence — this tier would go green having + /// established nothing about whether it ran its identity fixtures at all, which is the exact + /// failure mode the tier exists to remove. + /// + [TestMethod] + public void Not_Applicable_Trailer_Absent_Is_Not_Zero() + { + const string run = "# Running: F\nok F_Check\n# Total failures: 0\n"; + + var missing = PackagedSelfTestBatch.ExtractNotApplicableFixtures(run); + var zero = PackagedSelfTestBatch.ExtractNotApplicableFixtures( + run + PackagedSelfTestBatch.NotApplicableCountMarker + "0\n"); + + Assert.IsNull(missing.Count, "Absent trailer must not be reported as a count."); + Assert.AreEqual(0, zero.Count, "An explicit zero is a measurement, not an absence."); + } + + /// + /// Names have to survive, because the failure message is the only thing that tells a reader + /// which fixtures the packaged host decided it could not run — and the answer is + /// almost certainly the identity set. + /// + [TestMethod] + public void Not_Applicable_Trailer_Names_Are_Parsed() + { + var report = PackagedSelfTestBatch.ExtractNotApplicableFixtures( + PackagedSelfTestBatch.NotApplicableCountMarker + "2\n" + + PackagedSelfTestBatch.NotApplicableListMarker + + "Packaged_IdentityGuard, Packaged_SettingsStoreRoundTrip\n"); + + Assert.AreEqual(2, report.Count); + CollectionAssert.AreEqual( + new[] { "Packaged_IdentityGuard", "Packaged_SettingsStoreRoundTrip" }, + report.Names.ToArray()); + } + + /// + /// The skip trailers sit two lines above the exclusion trailer in the same stream and share + /// its shape. Reading one as the other would let a run with a single skipped fixture report a + /// non-zero exclusion count and redden this tier for the wrong reason. + /// + [TestMethod] + public void Skip_Trailers_Are_Not_The_Not_Applicable_Trailer() + { + var report = PackagedSelfTestBatch.ExtractNotApplicableFixtures( + "# Total skipped fixtures: 1\n# Skipped fixture list: Some_Fixture\n"); + + Assert.IsNull(report.Count); + } + + /// + /// A garbled count must not default to zero: zero is the answer this tier wants to hear, so a + /// malformed line must not be able to supply it. + /// + [TestMethod] + public void Not_Applicable_Malformed_Count_Is_Not_Zero() + { + var report = PackagedSelfTestBatch.ExtractNotApplicableFixtures( + PackagedSelfTestBatch.NotApplicableCountMarker + "several\n"); + + Assert.IsNull(report.Count); + } + // ── Host-directory override ──────────────────────────────────────── /// diff --git a/tests/Reactor.PackagedTests/PackagedSelfTestBatch.cs b/tests/Reactor.PackagedTests/PackagedSelfTestBatch.cs index bf2ef4b3d..229cb8ad0 100644 --- a/tests/Reactor.PackagedTests/PackagedSelfTestBatch.cs +++ b/tests/Reactor.PackagedTests/PackagedSelfTestBatch.cs @@ -53,6 +53,25 @@ internal enum FixtureStatus { Passed, Failed, Skipped } internal sealed record FixtureOutcome(FixtureStatus Status, string Detail); + /// + /// The host's # Total not-applicable fixtures: trailer. Duplicated from + /// SelfTestRunner.NotApplicableCountMarker, which this project cannot reference. + /// + internal const string NotApplicableCountMarker = "# Total not-applicable fixtures: "; + + /// + /// The host's # Not applicable fixture list: trailer. Duplicated from + /// SelfTestRunner.NotApplicableListMarker. + /// + internal const string NotApplicableListMarker = "# Not applicable fixture list: "; + + /// + /// What the host said it deliberately did not run. A + /// means the trailer was absent, which is not the same fact as a + /// count of zero: absent means nothing reported, zero means everything applied. + /// + internal sealed record NotApplicableReport(int? Count, IReadOnlyList Names); + // ──────────────────────────────────────────────────────────────────── // Run // ──────────────────────────────────────────────────────────────────── @@ -219,6 +238,88 @@ public void IdentityDependentFixtures_Actually_Asserted() "host lacked package identity or the gate itself is broken."); } + /// + /// The mirror of SelfTestBatch.NotApplicableFixtures_AreExcludedFromThisTier, and the + /// half that guards the dangerous direction. + /// + /// + /// Identity-dependent fixtures are declared SelfTestTier.Packaged in + /// SelfTestFixtureRegistry.TierRequirements, so the unpackaged host filters them out of + /// its corpus entirely (issue #1154). That filter keys off the same entry-assembly probe the + /// fixtures' own gate uses — and if it ever answered "unpackaged" here, this tier would + /// silently drop the only fixtures it exists to run, discover a corpus without them, and go + /// green having measured nothing. No other assertion catches that: they would not be missing + /// results, they would not be skips, they simply would not be part of the run, so + /// is never asked about them and + /// finds nothing to complain + /// about. + /// Hence the assertion is on the count the host itself reports, not on the discovered + /// set: the trailer is emitted before any filtering this shim does, so it describes the host's + /// own view of its corpus. + /// + [TestMethod] + public void EveryFixture_IsApplicableToThePackagedTier() + { + FailIfNotInitialized(); + + var report = ExtractNotApplicableFixtures(_fullOutput); + + Assert.IsNotNull( + report.Count, + $"The packaged host emitted no '{NotApplicableCountMarker.Trim()}' trailer, so nothing " + + "establishes that it considered its identity-dependent fixtures applicable. Either " + + "SelfTestRunner.WriteNotApplicableTrailer stopped being called, or the marker literal " + + "drifted between the host and this file — they are duplicated, not shared, because " + + $"the host is referenced with ReferenceOutputAssembly=false.\n{Tail(_fullOutput, 2000)}"); + + Assert.AreEqual( + 0, report.Count!.Value, + "The packaged host excluded fixtures from its own corpus as 'not applicable to this " + + $"tier':\n {string.Join("\n ", report.Names)}\n" + + "Every fixture must be applicable here — this is the tier that runs the " + + $"'{IdentityFixturePrefix}' set for real. The most likely cause is " + + "SelfTestFixtureRegistry.CurrentTier resolving to Unpackaged inside the packaged host, " + + "which would remove exactly those fixtures and leave the run green having measured " + + "nothing."); + } + + /// + /// Reads the host's # Total not-applicable fixtures: / # Not applicable fixture + /// list: trailers. Marker literals duplicated from SelfTestRunner. + /// + /// + /// The count is parsed independently of the list rather than derived from it, so a stream + /// truncated between the two lines is visible as a disagreement instead of silently reading + /// as an empty exclusion set — which is the answer this tier wants to hear. + /// + internal static NotApplicableReport ExtractNotApplicableFixtures(string stdout) + { + if (string.IsNullOrEmpty(stdout)) return new NotApplicableReport(null, []); + + int? count = null; + string[] names = []; + + foreach (var line in stdout.Split('\n').Select(static raw => raw.Trim())) + { + if (line.StartsWith(NotApplicableCountMarker, StringComparison.Ordinal)) + { + if (int.TryParse(line[NotApplicableCountMarker.Length..].Trim(), + System.Globalization.NumberStyles.Integer, + System.Globalization.CultureInfo.InvariantCulture, out var parsed)) + { + count = parsed; + } + } + else if (line.StartsWith(NotApplicableListMarker, StringComparison.Ordinal)) + { + names = line[NotApplicableListMarker.Length..] + .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } + } + + return new NotApplicableReport(count, names); + } + /// /// Surfaces any fixture whose result was parsed but which has no [DynamicData] test /// case to report it. diff --git a/tests/Reactor.SelfTests/SelfTestBatch.cs b/tests/Reactor.SelfTests/SelfTestBatch.cs index 6d0987f6e..ef1c205bf 100644 --- a/tests/Reactor.SelfTests/SelfTestBatch.cs +++ b/tests/Reactor.SelfTests/SelfTestBatch.cs @@ -1266,6 +1266,178 @@ public void SkippedFixtures_AreReported() /// internal const string SkipVerdictControlFixture = "SelfTestVerdict_OnlySkips_PositiveControl"; + // ════════════════════════════════════════════════════════════════════ + // Tier applicability (issue #1154) + // ════════════════════════════════════════════════════════════════════ + + /// + /// The Host's # Total not-applicable fixtures: trailer. Duplicated from + /// SelfTestRunner.NotApplicableCountMarker, which cannot be referenced from here. + /// + internal const string NotApplicableCountMarker = "# Total not-applicable fixtures: "; + + /// + /// The Host's # Not applicable fixture list: trailer. Duplicated from + /// SelfTestRunner.NotApplicableListMarker. + /// + internal const string NotApplicableListMarker = "# Not applicable fixture list: "; + + /// + /// What the Host said it deliberately did not run. is + /// when the trailer was absent entirely, which is a different fact from + /// a count of zero: absent means the mechanism is silent, zero means it ran everything. + /// + internal sealed record NotApplicableReport(int? Count, IReadOnlyList Names); + + /// + /// Reads the Host's tier-exclusion trailer. + /// + /// + /// Last-wins on both markers, for the same reason is: + /// a re-entered Host can emit the trailer twice and the final one describes the run being + /// reported on. The count is parsed independently of the list so the two can be compared + /// — a truncated stream that drops the list line while keeping the count is exactly the case + /// worth catching, and a parser that derived the count from the list could not see it. + /// + internal static NotApplicableReport ExtractNotApplicableFixtures(string stdout) + { + if (string.IsNullOrEmpty(stdout)) return new NotApplicableReport(null, []); + + int? count = null; + string[] names = []; + + foreach (var line in stdout.Split('\n', StringSplitOptions.TrimEntries)) + { + if (line.StartsWith(NotApplicableCountMarker, StringComparison.Ordinal)) + { + if (int.TryParse(line[NotApplicableCountMarker.Length..].Trim(), + System.Globalization.NumberStyles.Integer, + System.Globalization.CultureInfo.InvariantCulture, out var parsed)) + { + count = parsed; + } + } + else if (line.StartsWith(NotApplicableListMarker, StringComparison.Ordinal)) + { + names = line[NotApplicableListMarker.Length..] + .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } + } + + return new NotApplicableReport(count, names); + } + + /// + /// Proves the tier filter actually removed the fixtures this host cannot run, rather than the + /// fixtures having quietly ceased to exist. + /// + /// Why an absence needs an assertion. Issue #1154's fix is a removal: three + /// Packaged_* fixtures used to run here, self-skip, and put a permanent amber entry in + /// . They now aren't selected at all. The trouble with + /// fixing something by removal is that success and catastrophe produce the identical + /// observation — "no amber" is what you get whether the filter works or whether somebody + /// deleted the packaged corpus. Nothing else in this suite can tell those apart: the packaged + /// tier is a separate project in a separate CI job, and every other test here reasons about + /// fixtures that did report. + /// + /// So this asserts the mechanism positively, in the tier where it fires. This wrapper + /// always drives the unpackaged Host, so a non-empty exclusion set is deterministic + /// rather than machine-dependent — there is no configuration in which zero is the healthy + /// answer here. + /// + /// The mirror assertion lives in PackagedSelfTestBatch and demands the opposite: + /// zero exclusions there. Together they pin the filter in both directions, which matters + /// because a tier probe stuck on one answer would otherwise look correct from whichever side + /// happened to agree with it. + /// + [TestMethod] + public void NotApplicableFixtures_AreExcludedFromThisTier() + { + Assert.IsTrue(_initialized, "Self-test batch did not run."); + if (_initError is not null) + Assert.Fail(_initError); + + if (_timedOut || _abortedReason is not null) + { + Assert.Inconclusive( + $"{_abortedReason ?? "The suite was killed by its process budget"}; the Host never " + + $"reached its trailers, so the absence of the exclusion report says nothing about " + + $"the tier filter."); + } + + var report = ExtractNotApplicableFixtures(_fullOutput); + + Assert.IsNotNull(report.Count, + $"The Host completed its run but emitted no '{NotApplicableCountMarker.Trim()}' " + + $"trailer, so nothing reports which fixtures this tier declined to run. Either " + + $"SelfTestRunner.WriteNotApplicableTrailer stopped being called, or the marker literal " + + $"drifted between SelfTestRunner and this file — they are duplicated, not shared, " + + $"because the Host is referenced with ReferenceOutputAssembly=false."); + + Assert.AreEqual(report.Count!.Value, report.Names.Count, + $"The Host reported {report.Count} not-applicable fixture(s) but named " + + $"{report.Names.Count}. The TAP stream is inconsistent with itself — most likely " + + $"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, + $"This wrapper always runs the UNPACKAGED Host, where the '{PackagedFixturePrefix}' " + + $"fixtures are declared SelfTestTier.Packaged and must therefore be excluded — so zero " + + $"is never the healthy answer here. Something removed the mechanism or its subject:\n" + + $" (a) SelfTestFixtureRegistry.TierRequirements was emptied, so nothing is declared " + + $"tier-specific any more and those fixtures are back to self-skipping into the amber " + + $"inventory (issue #1154 restored).\n" + + $" (b) SelfTestFixtureRegistry.CurrentTier mis-evaluates — if it answered 'Packaged' " + + $"here, the filter would be a no-op in both tiers while still looking like it worked.\n" + + $" (c) The packaged fixtures were deleted. Their coverage is the only thing that runs " + + $"under real MSIX identity; restore them rather than this assertion."); + + // Discovery and the run must agree. `--list-fixtures` and the run list are fed from the + // same registry property precisely so they cannot diverge, and this is what would catch + // it if one of them were repointed at the unfiltered corpus: an excluded fixture that + // still had a `[TestMethod]` would report "was not reported by the Host" — a red that + // names the wrong cause entirely. + var discovered = new HashSet(FixtureNames.Value, StringComparer.Ordinal); + var leakedIntoDiscovery = report.Names.Where(discovered.Contains).ToArray(); + + Assert.AreEqual(0, leakedIntoDiscovery.Length, + $"`--list-fixtures` offered fixture(s) the run then declared not applicable:\n " + + $"{string.Join("\n ", leakedIntoDiscovery)}\n" + + $"Discovery and execution must be fed from the same tier-filtered set " + + $"(SelfTestFixtureRegistry.FixturesForCurrentTier), or each of these gets a test case " + + $"that can only ever fail for want of a result."); + + var leakedIntoRun = report.Names.Where(_byFixture.ContainsKey).ToArray(); + + Assert.AreEqual(0, leakedIntoRun.Length, + $"Fixture(s) declared not applicable to this tier nonetheless produced TAP output:\n " + + $"{string.Join("\n ", leakedIntoRun)}\n" + + $"The trailer and the run disagree, so one of them is lying about what executed."); + + // Informational, not a warning: this is the channel that replaces what the amber used to + // carry, and the whole point of the change is that a structural exclusion is not a + // finding. Best-effort because the assertions above are the load-bearing half — unlike the + // duration and skip reports, nothing here is silent without it. + var markdown = + $"### ℹ️ Selftest fixtures not applicable to this tier\n\n" + + $"{report.Count} fixture(s) were excluded from this run because they require a " + + $"different host, and were not executed:\n\n" + + string.Join("", report.Names.Select(n => $"- `{n}`\n")) + + $"\nDeclared in `SelfTestFixtureRegistry.TierRequirements`. These are asserted " + + $"for real by `Reactor.PackagedTests`. Background: issue #1154."; + + Console.WriteLine($"Not applicable to this tier ({report.Count}): {string.Join(", ", report.Names)}"); + PublishBestEffort( + () => TryAppendSummary(Environment.GetEnvironmentVariable(StepSummaryEnvVar), markdown), + "the tier-exclusion report"); + } + + /// + /// Naming convention for identity-dependent fixtures, quoted in the diagnostic above so it + /// points at a real set of names. Mirrors PackagedSelfTestBatch.IdentityFixturePrefix. + /// + internal const string PackagedFixturePrefix = "Packaged_"; + /// /// The one assertion that observes the real Host's real skip output, and the only thing /// that can catch the two projects drifting apart. @@ -1628,10 +1800,7 @@ private static string[] LoadFixtureNames() throw new InvalidOperationException( $"`--list-fixtures` failed with exit code {exitCode}.\nstdout:\n{stdout}\nstderr:\n{stderr}"); - var names = stdout.Split('\n', StringSplitOptions.RemoveEmptyEntries) - .Select(l => l.Trim()) - .Where(l => l.Length > 0) - .ToArray(); + var names = ParseFixtureNames(stdout); if (names.Length == 0) throw new InvalidOperationException( @@ -1640,6 +1809,25 @@ private static string[] LoadFixtureNames() return names; } + /// + /// Reduces a --list-fixtures dump to fixture names. + /// + /// + /// TAP comments are not fixture names. The Host appends its tier-exclusion trailer to this + /// stream (issue #1154), and --list-fixtures is free to grow further comments; treating + /// one as a name would manufacture a [TestMethod] for a fixture that cannot exist, which + /// would then fail as "was not reported by the Host" — a red naming the wrong cause entirely. + /// Mirrors PackagedSelfTestBatch.ParseFixtureList, which has always filtered them. + /// Split out from so the filter is testable without + /// launching a Host: the whole point of a guard against a malformed line is that it holds for + /// lines the current Host does not happen to emit. + /// + internal static string[] ParseFixtureNames(string stdout) => + stdout.Split('\n', StringSplitOptions.RemoveEmptyEntries) + .Select(l => l.Trim()) + .Where(l => l.Length > 0 && !l.StartsWith('#')) + .ToArray(); + // -- Process runner: async reads + timeout race with kill ------------------ private static (string Stdout, string Stderr, int ExitCode, bool TimedOut) RunProcess( diff --git a/tests/Reactor.SelfTests/TierApplicabilityTests.cs b/tests/Reactor.SelfTests/TierApplicabilityTests.cs new file mode 100644 index 000000000..40d0f44a5 --- /dev/null +++ b/tests/Reactor.SelfTests/TierApplicabilityTests.cs @@ -0,0 +1,198 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.UI.Reactor.SelfTests; + +/// +/// Headless guards for the tier-applicability reporting added for issue #1154. Everything here is +/// pure, so this class does not launch the Host and does not trigger 's +/// [ClassInitialize]. +/// +/// What is being defended. Three Packaged_* fixtures used to run in the +/// unpackaged host, self-skip, and leave a permanent amber entry in +/// SkippedFixtures_AreReported — a channel whose value depends on being rare. They are now +/// declared SelfTestTier.Packaged and are not selected here at all. That is a fix by +/// removal, and the trouble with those is that success and catastrophe look identical from +/// outside: "no amber" is what you get whether the filter works or whether somebody deleted the +/// packaged corpus. The Host therefore names what it excluded, and +/// NotApplicableFixtures_AreExcludedFromThisTier asserts on it. These tests pin the parser +/// that assertion depends on. +/// +/// The distinction that carries the weight is absent trailer versus zero +/// exclusions. Absent means the reporting mechanism is silent; zero means the host ran +/// everything. Collapsing them would make a Host that stopped emitting the trailer entirely +/// indistinguishable from a healthy packaged run — and the packaged shim's mirror assertion +/// demands exactly zero, so it would then pass on silence. Several tests below exist only to keep +/// those two apart. +/// +[TestClass] +public class TierApplicabilityTests +{ + private const string Count = SelfTestBatch.NotApplicableCountMarker; + private const string List = SelfTestBatch.NotApplicableListMarker; + + private const string RunBody = + "TAP version 14\n1..2\n# Running: A\nok A_Check\n# Running: B\nok B_Check\n# Total failures: 0\n"; + + // ------------------------------------------------------------------ the happy path + + /// + /// Stated as a differential over the same stream, so a parser that hard-coded the packaged + /// fixture names — or returned a canned list for anything — fails here. Only the trailer + /// differs between the two arms. + /// + [TestMethod] + public void TrailerNamesTheExcludedFixtures() + { + var without = SelfTestBatch.ExtractNotApplicableFixtures(RunBody); + var with = SelfTestBatch.ExtractNotApplicableFixtures( + RunBody + $"{Count}2\n{List}Packaged_IdentityGuard, Packaged_SettingsStoreRoundTrip\n"); + + Assert.IsNull(without.Count, "No trailer in the stream, so there is nothing to report."); + Assert.AreEqual(2, with.Count); + CollectionAssert.AreEqual( + new[] { "Packaged_IdentityGuard", "Packaged_SettingsStoreRoundTrip" }, + with.Names.ToArray(), + "The names are what the assertion cross-checks against discovery and against the " + + "parsed results, so they have to survive the comma split with their whitespace " + + "trimmed."); + } + + // ------------------------------------------------------------------ absent vs zero + + /// + /// The load-bearing distinction. PackagedSelfTestBatch asserts the count is zero + /// in the packaged tier; if a missing trailer parsed as zero, a Host that stopped reporting + /// altogether would satisfy that assertion by silence — the tier would go green having + /// established nothing about its own corpus, which is the failure mode the packaged tier + /// exists to remove. + /// + [TestMethod] + public void MissingTrailer_IsNotTheSameAsZeroExclusions() + { + var missing = SelfTestBatch.ExtractNotApplicableFixtures(RunBody); + var zero = SelfTestBatch.ExtractNotApplicableFixtures(RunBody + $"{Count}0\n"); + + Assert.IsNull(missing.Count, "Absent trailer must not be reported as a count."); + Assert.AreEqual(0, zero.Count, "An explicit zero is a measurement, not an absence."); + Assert.AreEqual(0, zero.Names.Count, "A zero count legitimately carries no list line."); + } + + /// + /// A count that cannot be parsed must stay rather than default to zero, + /// for the same reason: zero is the answer the packaged tier wants to hear, so a malformed + /// line must not be able to supply it. + /// + [TestMethod] + public void MalformedCount_DoesNotMasqueradeAsZero() + { + var report = SelfTestBatch.ExtractNotApplicableFixtures(RunBody + $"{Count}three\n"); + + Assert.IsNull(report.Count, + "'three' is not a count. Reading it as 0 would let a garbled trailer assert that " + + "every fixture applied to this tier."); + } + + // ------------------------------------------------------------------ truncation + + /// + /// The count is parsed independently of the list precisely so the two can be compared. A + /// stream cut between the two trailer lines is a realistic failure here — the Host ends its + /// run with a teardown-free TerminateProcess (issue #680) — and a parser that derived + /// the count from the list would report a consistent, wrong, smaller answer instead. + /// + [TestMethod] + public void CountWithoutItsList_IsVisibleAsADisagreement() + { + var report = SelfTestBatch.ExtractNotApplicableFixtures(RunBody + $"{Count}3\n"); + + Assert.AreEqual(3, report.Count); + Assert.AreEqual(0, report.Names.Count, + "The list line never arrived, and the count must not be back-filled from it — the " + + "gap is the finding."); + } + + /// + /// A re-entered Host can emit the trailer twice; the final one describes the run being + /// reported on. Same last-wins rule as ExtractSuiteElapsedSeconds. + /// + [TestMethod] + public void LastTrailerWins() + { + var report = SelfTestBatch.ExtractNotApplicableFixtures( + $"{Count}1\n{List}Stale_Fixture\n" + RunBody + $"{Count}2\n{List}Real_One, Real_Two\n"); + + Assert.AreEqual(2, report.Count); + CollectionAssert.AreEqual(new[] { "Real_One", "Real_Two" }, report.Names.ToArray()); + } + + /// + /// A malformed later marker must not discard a value already parsed, or a stray line could + /// silence the whole report. Mirrors the rule SuiteElapsed_LastParseableMarkerWins + /// pins for the duration trailer. + /// + [TestMethod] + public void MalformedLaterMarker_DoesNotDiscardAParsedCount() + { + var report = SelfTestBatch.ExtractNotApplicableFixtures( + $"{Count}2\n{List}Real_One, Real_Two\n" + RunBody + $"{Count}\n"); + + Assert.AreEqual(2, report.Count, + "An unparseable marker is skipped, not treated as a new answer."); + } + + // ------------------------------------------------------------------ marker isolation + + /// + /// The new prefixes share a shape with the skip trailers that sit two lines above them in the + /// same stream. TESTING.md states the rule that a grep for one must not match the others, and + /// this is where it is enforced: # Total skipped fixtures: and + /// # Skipped fixture list: must be inert here. + /// + [TestMethod] + public void SkipTrailers_AreNotMistakenForTheExclusionTrailer() + { + var report = SelfTestBatch.ExtractNotApplicableFixtures( + RunBody + + "# Total skipped fixtures: 1\n" + + "# Skipped fixture list: SelfTestVerdict_OnlySkips_PositiveControl\n"); + + Assert.IsNull(report.Count, + "The skip trailers were read as the exclusion trailer. Those two reports mean " + + "opposite things — one names fixtures that ran and established nothing, the other " + + "names fixtures that deliberately did not run."); + } + + // ------------------------------------------------------------------ discovery + + /// + /// Discovery reads the same stream the trailer is appended to, so a comment must not become a + /// fixture name. It would get a [TestMethod] of its own that could only ever fail as + /// "was not reported by the Host" — a red pointing at a fixture that does not exist. + /// + [TestMethod] + public void FixtureNameParsing_DropsTapComments() + { + var names = SelfTestBatch.ParseFixtureNames( + $"Alpha_One\nBeta_Two\n{Count}1\n{List}Packaged_IdentityGuard\n"); + + CollectionAssert.AreEqual(new[] { "Alpha_One", "Beta_Two" }, names, + "Only the two real names are fixtures; both trailer lines are TAP comments."); + } + + /// + /// The other direction: the comment filter must key on the comment marker, not on anything + /// resembling the excluded names, or a legitimately-named fixture could be dropped from + /// discovery and silently never run. + /// + [TestMethod] + public void FixtureNameParsing_KeepsNamesThatResembleTheTrailer() + { + var names = SelfTestBatch.ParseFixtureNames( + "Packaged_IdentityGuard\nNotApplicable_Fixture\nTotal_Fixture\n"); + + CollectionAssert.AreEqual( + new[] { "Packaged_IdentityGuard", "NotApplicable_Fixture", "Total_Fixture" }, names, + "These are fixture names, not comments — the packaged host lists the first one for " + + "real, so dropping it would empty the tier this whole mechanism exists to feed."); + } +}