Coverage gate: replace the global MaxSkipped ceiling with per-file skip attribution #132

Problem

The MaxSkipped ceiling in Tests/coverage-baseline.psd1 is a single global number per
edition. Because it is a ceiling with headroom rather than a pin, it silently absorbs skip
movement until some later, unrelated change crosses the line — and then it reds that change
instead of the one that caused the movement.

This is not hypothetical. It just happened, and the arithmetic is exact.

The incident

A local Windows PowerShell 5.1 full-suite run measured 292 skipped against the ceiling of
268, so the gate failed. Attributing that 292 by source file shows the +40 over the 252 that
originally justified 268 splits in two:

So #120 added 6 legitimate PS7-gated skips and never needed to touch the ceiling, because at
258 measured against 268 it still had slack. It passed. The ceiling was then stale — carrying
10 headroom, not the 16 its own comment claimed — and the next branch to cross the line was
billed for all 40, including #120's 6.

Why the headroom is the mechanism, not the mitigation

Headroom exists to avoid false reds. What it actually does is convert an attributable red
on the PR that caused the movement into an unattributable red on a later innocent PR. That
is strictly worse for whoever has to diagnose it: they get a number that is wrong by an amount
they did not cause, with no signal telling them which part is theirs.

It also means a real coverage regression of up to MaxSkipped - measured tests is invisible.
That is the issue #63 failure shape — green summary, lost coverage — just bounded in size.

Proposed design: per-file expected skips

Replace (or supplement) the global ceiling with a per-file map of expected skip counts.

ExpectedSkips = @{
    'Build-PfbApiDriftReport.Tests.ps1'  = 65
    'Build-PfbCapabilityMap.Tests.ps1'   = 50
    # ...
}

Assertion becomes: for every file, actual skips equal expected. Then

• a new PS7-gated file is an explicit new entry in the reviewed diff — the thing the
  maintainer actually wants to eyeball;
• an existing file that starts skipping reds immediately and names itself, instead of
  being absorbed by slack;
• a file that stops skipping also reds, which catches an accidentally-ungated test — something
  a ceiling structurally cannot see;
• blame lands on the causing PR, every time, so ceilings cannot go stale between merges.

This composes with the existing RequiredDescribes allowlist rather than replacing it. That
list catches a Describe vanishing from the result tree entirely (contributing neither a skip
nor a pass), which per-file skip counts still cannot see.

Data already gathered

Full per-file skip attribution from the 5.1 run described above (2983 passed / 0 failed / 292
skipped). The 19 entries sum to exactly 292, so this is complete, not a sample:

  65  Build-PfbApiDriftReport.Tests.ps1
  50  Build-PfbCapabilityMap.Tests.ps1
  34  Update-PfbTestModuleImport.Tests.ps1     <-- unmerged branch, not on main yet
  33  PfbPipelineSelectorTools.Tests.ps1
  16  PfbSelectorProbeHarness.Tests.ps1
  15  Build-PfbFieldCmdletMap.Tests.ps1
  14  Build-PfbValueEnumMap.Tests.ps1
  11  PfbPipelineSelectorRail.Tests.ps1
   9  Build-PfbResponseShapeMap.Tests.ps1
   8  Build-PfbCapabilityMap.ContextScopeDrift.Tests.ps1
   8  PfbApiDriftTools.Tests.ps1
   8  PfbValueEnumTools.Tests.ps1
   6  PfbSpecTools.Tests.ps1
   6  Build-PfbDeadKeyReport.Tests.ps1
   4  PfbSpecTools.ContextScope.Tests.ps1
   2  New-PfbJwtToken.Tests.ps1
   1  Set-PfbTlsProtocol.Tests.ps1
   1  Connect-PfbArray.OAuth2TokenRefresh.Tests.ps1
   1  Issue31.DriftConfidence.Tests.ps1

Corresponding pwsh7 leg for the same tree: 3273 passed / 0 failed / 2 skipped against its
ceiling of 8 — which is why these files being PS7-gated is correct behaviour rather than lost
coverage.

Caveat on provenance: both figures are local runs, not CI run numbers like the ones cited
in coverage-baseline.psd1 today. Expect CI on windows-latest to differ slightly. The
per-file shape is what matters here, not the absolute totals.

Implementation notes

No log parsing needed. Assert-PfbTestCoverage.ps1 already receives the result object, and
that object carries the file per test. Verified directly on Pester 6.0.1, both editions:

• $Result.Tests[].ScriptBlock.File — full path of the source file for each test.
• $Result.Containers[].Item — a FileInfo per container, so .Name gives the leaf.

Both are available alongside the existing .Path[0] (top-level Describe) and .Result that
the current assertions already use, so this needs no new plumbing in the gate's signature.

Gotchas worth knowing up front:

• Key the map on the leaf file name, not a full path. Paths differ between runners and
  between a normal checkout and a worktree.
• Tests/ currently has no test files in subdirectories, so leaf names are unique today. If
  that changes, a leaf-keyed map silently merges two files — worth a uniqueness assertion.
• Both baseline blocks (pwsh7 and winps51) need the treatment. They differ by a wide
  margin by design: the tooling Describes are PS7-gated, so the 5.1 leg skips essentially all
  of them.
• Import-PowerShellDataFile cannot parse a single collection literal with more than ~500
  entries. A ~195-entry hashtable per edition is well clear of that, but the two blocks should
  stay separate hashtables rather than being merged into one flat list.
• The gate script is constrained to Windows PowerShell 5.1 syntax — no ternaries, no ??, no
  &&/||. It runs under both editions.
• Deciding what to do about a file that is absent from the run entirely (filtered out, or
  deleted) is the interesting design question — that is the graceful-skip hole
  RequiredDescribes exists for, and a per-file skip map has the same blind spot unless
  absence is treated as a violation.

Reproducing the current numbers

The attribution above was produced by scraping a saved full-suite log, purely because the run
had already finished. This is a one-off diagnostic, not the shape the real implementation
should take — use the result object instead, as above. Included only so the numbers can be
re-derived from any existing log.

param([Parameter(Mandatory)][string]$LogPath)

# Pester writes ANSI colour codes, so lines do not start with the text they appear to start
# with. Strip them before any matching or every pattern silently fails to match.
$esc = [char]27
$lines = ((Get-Content $LogPath -Raw) -replace "$esc\[[0-9;]*m", '') -split "`r?`n"

$current = ''
$tally = @{}
foreach ($line in $lines) {
    $trimmed = $line.Trim()
    if ($trimmed -like "Running tests from '*") {
        $current = ($trimmed.TrimEnd("'") -split '[\\/]')[-1]
    }
    elseif ($trimmed.StartsWith('[!]') -and $current -ne '') {
        if (-not $tally.ContainsKey($current)) { $tally[$current] = 0 }
        $tally[$current] = $tally[$current] + 1
    }
}

"TOTAL = " + ($tally.Values | Measure-Object -Sum).Sum
$tally.GetEnumerator() | Sort-Object Value -Descending |
    ForEach-Object { '{0,4}  {1}' -f $_.Value, $_.Key }

Sanity check: the total must equal the run's reported skipped + not-run count. If it does not,
the boundary detection is wrong and the per-file numbers cannot be trusted.

Open questions for the maintainer

1. Replace the ceiling, or keep both? A global ceiling is a cheap backstop against a
   file-level map drifting out of date, but two overlapping assertions on the same quantity may
   not be worth the maintenance.
2. How noisy is acceptable? Exact per-file matching means any legitimate change to a gated
   test reds CI until the map is updated in the same PR. That is the point, but it is a real
   increase in required diff churn.
3. Absent files — violation, or ignored? See the last implementation note.

Cheap interim alternative

If the per-file map is more churn than wanted, setting MaxSkipped to the measured value with
zero headroom captures most of the benefit for a one-line change: movement then reds the PR
that caused it rather than a later one. It still cannot say which file moved, and it cannot
detect a test that stopped skipping.