diff --git a/Tests/CLAUDE.md b/Tests/CLAUDE.md index 91bffea..26b27ab 100644 --- a/Tests/CLAUDE.md +++ b/Tests/CLAUDE.md @@ -102,23 +102,43 @@ is CI's job and is not worth simulating. Two independent assertions guard against tests silently vanishing: -- **`MaxSkipped`** — a per-edition ceiling on skipped tests, catching a Describe - that starts skipping. It is a ceiling *with headroom*, not a pin. Raise it - deliberately, in a reviewed diff, and say why in the commit message. +- **`ExpectedSkips`** — a per-edition, per-**file** expected skipped-test count. + These are exact numbers, not ceilings. If a file's count moves in either + direction the gate reds and names the file; update that file's line in the + same diff and say why in the commit message. Only files that actually skip + appear (19 of 199 on 5.1, 2 on pwsh 7), so it is a short list rather than a + census. - **`RequiredDescribes`** — names Describes that must appear in the result tree. Blocks that skip *gracefully* (their guard evaluates false at `BeforeAll` - time) contribute neither a pass nor a skip, so a skip ceiling cannot see them + time) contribute neither a pass nor a skip, so a skip count cannot see them disappear at all. +Three rails come with the map, in `scripts/Assert-PfbTestCoverage.ps1`: no test +file may run **empty** (contributing neither an executed nor a skipped test), no +**undeclared** file may skip, and a declared file that stops running at all is a +violation rather than a stale entry to delete. + The two editions differ by a wide margin — every Describe in the tooling test files carries `-Skip:($PSVersionTable.PSVersion.Major -lt 7)`, so the 5.1 leg -skips all of them by design. One shared ceiling would be either a permanent -false red on 5.1 or useless on 7. +skips all of them by design. One shared map would be either a permanent false +red on 5.1 or useless on 7. + +**Why exact and not a ceiling.** `ExpectedSkips` replaced a single global +`MaxSkipped` ceiling per edition (issue #132). That ceiling carried deliberate +headroom, and the headroom was the defect rather than the mitigation: it did not +prevent false reds, it converted an *attributable* red on the PR that moved the +number into an *unattributable* red on a later, innocent PR — and it hid a real +coverage regression of up to `ceiling - measured` tests, the issue-#63 failure +shape merely bounded in size. It happened once with exact arithmetic: a 5.1 run +measured 292 against a ceiling of 268, and six of the +40 belonged to #120 two +days earlier, which had slack and passed without touching the baseline. Do not +reintroduce headroom, globally or per file. One non-obvious way this gate reds without your branch changing: a -`pull_request` run tests the **merge** commit, so a ceiling measured before an +`pull_request` run tests the **merge** commit, so a map measured before an unrelated merge to `main` can go stale on its own. That is worth a red build, -not a suppression. +not a suppression — and it is now attributed to the file the merge moved, which +is the whole improvement. ## Tooling tests need the spec cache diff --git a/Tests/CiCoverageGate.Tests.ps1 b/Tests/CiCoverageGate.Tests.ps1 index 0f12f8c..b8ee593 100644 --- a/Tests/CiCoverageGate.Tests.ps1 +++ b/Tests/CiCoverageGate.Tests.ps1 @@ -27,10 +27,42 @@ BeforeAll { return [pscustomobject]@{ Path = $Path; Result = $Result } } + # The fake container paths must be PLATFORM-NATIVE, and both halves of that matter. A + # hardcoded 'C:\repo\Tests' broke the ubuntu and macos legs two ways at once: Join-Path + # throws DriveNotFoundException for a drive letter that does not exist, and even as a bare + # literal, Split-Path -Leaf does not treat '\' as a separator off Windows -- so the gate + # would key attribution on the whole string and every leaf lookup would silently miss. + # Building from the temp path keeps the root real enough for Join-Path on every platform + # while staying obviously fake; nothing here is ever created on disk. + $script:fakeTestsDir = Join-Path (Join-Path ([System.IO.Path]::GetTempPath()) 'pfb-fake-repo') 'Tests' + + # One entry per test FILE, mirroring $Result.Containers. The .Item shape matters: on a real + # run it is a FileInfo, and the gate reads .FullName off it, so the stand-in carries a + # FullName rather than a bare string. + function New-FakeContainer { + param( + [string]$File, + [int]$Passed = 0, [int]$Failed = 0, [int]$Skipped = 0, [int]$NotRun = 0, + [string]$Dir = $script:fakeTestsDir + ) + return [pscustomobject]@{ + Item = [pscustomobject]@{ FullName = (Join-Path $Dir $File) } + PassedCount = $Passed + FailedCount = $Failed + SkippedCount = $Skipped + NotRunCount = $NotRun + } + } + function New-FakeResult { - param([object[]]$Tests, [int]$Passed = 0, [int]$Failed = 0, [int]$Skipped = 0, [int]$NotRun = 0) + param( + [object[]]$Tests, + [object[]]$Containers, + [int]$Passed = 0, [int]$Failed = 0, [int]$Skipped = 0, [int]$NotRun = 0 + ) return [pscustomobject]@{ Tests = $Tests + Containers = $Containers PassedCount = $Passed FailedCount = $Failed SkippedCount = $Skipped @@ -38,18 +70,31 @@ BeforeAll { } } + # The two Describes every fake run needs in order to clear assertion 1, so that assertion + # 2's own failures are the only thing under test. + function New-HealthyDescribeTests { + return @( + (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed'), + (New-FakeTest -Path @('Block B', 'does another') -Result 'Passed') + ) + } + $script:baselineFile = Join-Path $TestDrive 'baseline.psd1' @' @{ pwsh7 = @{ - MaxSkipped = 5 + ExpectedSkips = @{ + 'Gated.Tests.ps1' = 3 + } RequiredDescribes = @( 'Block A' 'Block B' ) } winps51 = @{ - MaxSkipped = 100 + ExpectedSkips = @{ + 'Gated.Tests.ps1' = 50 + } RequiredDescribes = @('Block A') } } @@ -57,61 +102,157 @@ BeforeAll { } Describe 'Assert-PfbTestCoverage (issue #63 coverage gate)' { - It 'passes when every required Describe ran and the skip count is under the ceiling' { - $result = New-FakeResult -Passed 2 -Skipped 1 -Tests @( - (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed'), - (New-FakeTest -Path @('Block B', 'does another') -Result 'Passed') + + It 'passes when every required Describe ran and every file matches its expected skip count' { + $result = New-FakeResult -Passed 2 -Skipped 3 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3) ) { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | Should -Not -Throw } It 'FAILS when a required Describe contributed no executed tests at all' { - # THE case this gate exists for, and the one a skip ceiling cannot see. A Describe + # THE case assertion 1 exists for, and the one a skip count cannot see. A Describe # filtered out of the run, or guarded false at BeforeAll time, contributes NEITHER a - # skip NOR a pass. Note SkippedCount is 0 here: a ceiling-only gate reads this run as - # perfectly healthy while an entire required block is silently absent. - $result = New-FakeResult -Passed 1 -Skipped 0 -Tests @( + # skip NOR a pass. Note the per-file numbers here are all correct: a skip-count-only + # gate reads this run as perfectly healthy while an entire required block is absent. + $result = New-FakeResult -Passed 1 -Skipped 3 -Tests @( (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed') + ) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 1), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3) ) { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | Should -Throw -ExpectedMessage '*coverage gate failed*' } It 'FAILS when a required Describe is present but every test in it skipped' { - $result = New-FakeResult -Passed 1 -Skipped 1 -Tests @( + $result = New-FakeResult -Passed 1 -Skipped 4 -Tests @( (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed'), (New-FakeTest -Path @('Block B', 'does another') -Result 'Skipped') + ) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 1 -Skipped 1), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3) ) { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | Should -Throw -ExpectedMessage '*coverage gate failed*' } - It 'FAILS when the skip count exceeds the ceiling, even with every required Describe green' { - $result = New-FakeResult -Passed 2 -Skipped 6 -Tests @( - (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed'), - (New-FakeTest -Path @('Block B', 'does another') -Result 'Passed') + It 'FAILS when a declared file skips MORE than its expected count, and names that file' { + $result = New-FakeResult -Passed 2 -Skipped 9 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 9) + ) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | + Should -Throw -ExpectedMessage '*coverage gate failed*' + } + + It 'FAILS when a declared file skips FEWER than its expected count' { + # The direction a CEILING structurally cannot see, and the reason issue #132 replaced + # one. A file quietly losing skipped tests is movement nobody decided on: either those + # tests now run (good, and the number should say so) or they stopped existing (bad). + # Under the old global ceiling both readings passed silently, and the unrecorded drop + # then became someone else's unattributable red once the total drifted back up. + $result = New-FakeResult -Passed 2 -Skipped 1 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 1) + ) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | + Should -Throw -ExpectedMessage '*coverage gate failed*' + } + + It 'FAILS when an UNDECLARED file skips anything at all' { + # A new test file arriving with PS7-gated Describes. Its skips must be declared, so the + # next reader can see whether the gating was deliberate. + $result = New-FakeResult -Passed 2 -Skipped 5 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3), + (New-FakeContainer -File 'BrandNew.Tests.ps1' -Skipped 2) + ) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | + Should -Throw -ExpectedMessage '*coverage gate failed*' + } + + It 'FAILS when a file runs EMPTY, contributing neither an executed nor a skipped test' { + # The issue-#63 shape below the granularity RequiredDescribes can reach: a throwing + # top-level BeforeAll, or a `#requires` the runner does not satisfy. Such a file still + # appears in .Containers with every count zero, which is the only reason this is + # detectable -- it contributes NO entries to .Tests at all. + $result = New-FakeResult -Passed 2 -Skipped 3 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3), + (New-FakeContainer -File 'DiedInBeforeAll.Tests.ps1') + ) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | + Should -Throw -ExpectedMessage '*coverage gate failed*' + } + + It 'FAILS when a declared file did not run at all, rather than treating it as a stale entry' { + # A declared file dropping out of discovery is lost coverage. The tempting reading -- + # "the entry is stale, delete it" -- is the one that turns a regression into a tidy-up. + $result = New-FakeResult -Passed 2 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2) ) { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | Should -Throw -ExpectedMessage '*coverage gate failed*' } - It 'counts NotRun toward the skip ceiling, not just Skipped' { + It 'counts NotRun toward a file''s number, not just Skipped' { # Pester reports a block skipped at discovery time as NotRun rather than Skipped. - # Counting only one of the two would leave half the regression invisible. - $result = New-FakeResult -Passed 2 -Skipped 3 -NotRun 3 -Tests @( - (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed'), - (New-FakeTest -Path @('Block B', 'does another') -Result 'Passed') + # Counting only one of the two would leave half the movement invisible. Here the two + # sum to the expected 3, so this run must PASS -- which is what proves NotRun is being + # counted rather than merely tolerated. + $result = New-FakeResult -Passed 2 -Skipped 1 -NotRun 2 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 1 -NotRun 2) + ) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | Should -Not -Throw + } + + It 'FAILS when per-file attribution does not reconcile with the run total' { + # Not a coverage assertion -- a check on the walk itself. If the containers do not + # account for every skip the run reported, then every per-file verdict above was + # judging a partial picture, and a green result would be meaningless. + $result = New-FakeResult -Passed 2 -Skipped 40 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3) + ) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | + Should -Throw -ExpectedMessage '*coverage gate failed*' + } + + It 'FAILS loudly when the result carries no Containers at all, rather than passing vacuously' { + # ANTI-VACUITY. An empty .Containers is the false-zero shape: nothing skipped anywhere, + # every rail satisfied, gate green, gate useless. + $result = New-FakeResult -Passed 2 -Tests (New-HealthyDescribeTests) -Containers @() + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | + Should -Throw -ExpectedMessage '*coverage gate failed*' + } + + It 'FAILS when two containers share a leaf name, since attribution by leaf is then unsound' { + # Tests/ is flat today and no two *.Tests.ps1 files share a leaf, which is what makes + # the leaf a safe key. If a subdirectory ever reintroduces a collision, one file's + # numbers would mask another's -- so the gate says so instead of overwriting silently. + $result = New-FakeResult -Passed 2 -Skipped 3 -Tests (New-HealthyDescribeTests) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 2), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 3), + # Same leaf, different parent -- so -Dir, not a hardcoded literal, or the collision + # this test exists to provoke never happens off Windows. + (New-FakeContainer -File 'Gated.Tests.ps1' -Dir (Join-Path $script:fakeTestsDir 'Nested')) ) { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | Should -Throw -ExpectedMessage '*coverage gate failed*' } - It 'applies the per-edition block, so winps51 tolerates what pwsh7 rejects' { - # The editions differ by roughly 200 skips by design: every tooling Describe is - # PS7-gated. A single shared ceiling would be either a permanent false red on 5.1 or + It 'applies the per-edition block, so winps51 accepts a count pwsh7 rejects' { + # The editions differ by roughly 300 skips by design: every tooling Describe is + # PS7-gated. A single shared map would be either a permanent false red on 5.1 or # useless on 7. $result = New-FakeResult -Passed 1 -Skipped 50 -Tests @( (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed') + ) -Containers @( + (New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 1), + (New-FakeContainer -File 'Gated.Tests.ps1' -Skipped 50) ) { & $gateScript -Result $result -Edition winps51 -BaselinePath $baselineFile } | Should -Not -Throw { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $baselineFile } | @@ -119,28 +260,81 @@ Describe 'Assert-PfbTestCoverage (issue #63 coverage gate)' { } It 'throws a clear error when the baseline file is missing rather than passing vacuously' { - $result = New-FakeResult -Passed 1 -Tests @() + $result = New-FakeResult -Passed 1 -Tests @() -Containers @() { & $gateScript -Result $result -Edition pwsh7 -BaselinePath (Join-Path $TestDrive 'nope.psd1') } | Should -Throw -ExpectedMessage '*not found*' } + + It 'throws when an edition block has no ExpectedSkips map at all' { + # A block carrying only RequiredDescribes would silently gate nothing on the skip axis. + # That is the #63 shape applied to the gate's own configuration, so it is a hard throw + # rather than a violation. + $bad = Join-Path $TestDrive 'no-expected-skips.psd1' + @' +@{ + pwsh7 = @{ RequiredDescribes = @('Block A') } + winps51 = @{ RequiredDescribes = @('Block A') } +} +'@ | Set-Content -Path $bad -Encoding UTF8 + $result = New-FakeResult -Passed 1 -Tests @( + (New-FakeTest -Path @('Block A', 'does a thing') -Result 'Passed') + ) -Containers @((New-FakeContainer -File 'Ungated.Tests.ps1' -Passed 1)) + { & $gateScript -Result $result -Edition pwsh7 -BaselinePath $bad } | + Should -Throw -ExpectedMessage '*ExpectedSkips*' + } } Describe 'coverage-baseline.psd1 (the real one)' { - It 'parses, and defines both editions with a ceiling and a non-empty allowlist' { + It 'parses, and defines both editions with a non-empty skip map and a non-empty allowlist' { $real = Import-PowerShellDataFile -Path (Join-Path $repoRoot 'Tests/coverage-baseline.psd1') foreach ($edition in @('pwsh7', 'winps51')) { $real[$edition] | Should -Not -BeNullOrEmpty -Because "$edition must have a baseline block" - $real[$edition].MaxSkipped | Should -BeGreaterOrEqual 0 + $real[$edition].ExpectedSkips | Should -Not -BeNullOrEmpty -Because "$edition must declare a per-file expected-skip map (issue #132); a block without one gates nothing on the skip axis" @($real[$edition].RequiredDescribes).Count | Should -BeGreaterThan 0 -Because 'an empty allowlist is a gate that checks nothing' } } + It 'declares no MaxSkipped, in either edition' { + # Issue #132 removed the global ceiling because its headroom converted an attributable + # red into an unattributable one. A key left or reinstated here reads as configuration + # the gate honours, and nothing does -- so it would be a silent no-op that looks like + # a second layer of protection. Fail rather than ignore it. + $real = Import-PowerShellDataFile -Path (Join-Path $repoRoot 'Tests/coverage-baseline.psd1') + foreach ($edition in @('pwsh7', 'winps51')) { + $real[$edition].ContainsKey('MaxSkipped') | + Should -BeFalse -Because "the $edition block still declares MaxSkipped, which the gate no longer reads (issue #132). Per-file ExpectedSkips replaced it; a leftover key is a no-op masquerading as a gate." + } + } + + It 'names only test files that actually exist in Tests/' { + # The static half of the gate's own runtime check. A renamed or deleted test file + # leaving a stale entry behind would red CI for a reason that reads as mysterious, and + # this catches it locally, on a scoped run, before it costs a CI cycle. + $real = Import-PowerShellDataFile -Path (Join-Path $repoRoot 'Tests/coverage-baseline.psd1') + foreach ($edition in @('pwsh7', 'winps51')) { + foreach ($file in @($real[$edition].ExpectedSkips.Keys)) { + Test-Path -Path (Join-Path $repoRoot (Join-Path 'Tests' $file)) | + Should -BeTrue -Because "the $edition ExpectedSkips map names a test file that does not exist: '$file'" + } + } + } + + It 'declares only positive counts, since a zero entry is indistinguishable from omitting it' { + $real = Import-PowerShellDataFile -Path (Join-Path $repoRoot 'Tests/coverage-baseline.psd1') + foreach ($edition in @('pwsh7', 'winps51')) { + foreach ($file in @($real[$edition].ExpectedSkips.Keys)) { + [int]$real[$edition].ExpectedSkips[$file] | + Should -BeGreaterThan 0 -Because "the $edition entry for '$file' is not positive; an undeclared file is already required to skip nothing, so a zero entry only adds a line to maintain" + } + } + } + It 'registers the ungated value-enum citation guard in BOTH editions' { # The reverse direction of the check below, for one block that needs it. The # citation guard in Build-PfbValueEnumMap.Tests.ps1 carries no -Skip and reads only # committed .ps1 files, so it must contribute executed tests on every leg. If it is # filtered out, renamed, or its BeforeAll starts throwing silently, it contributes - # neither a pass nor a skip -- MaxSkipped cannot see that, and the build stays green + # neither a pass nor a skip -- a skip count cannot see that, and the build stays green # with the guard gone. Only RequiredDescribes catches it, so the entry is part of the # guard, not bookkeeping. $real = Import-PowerShellDataFile -Path (Join-Path $repoRoot 'Tests/coverage-baseline.psd1') diff --git a/Tests/CommittedDeadKeyReport.Tests.ps1 b/Tests/CommittedDeadKeyReport.Tests.ps1 index 6a3bc29..f79983c 100644 --- a/Tests/CommittedDeadKeyReport.Tests.ps1 +++ b/Tests/CommittedDeadKeyReport.Tests.ps1 @@ -120,7 +120,19 @@ BeforeAll { 'Remove-PfbNodeGroupNode|DELETE|node-groups/nodes' ) # CEILINGS, not pins -- see the monotone note above. - $script:baselineDeadKeyCount = 85 + # + # LOWERED 85 -> 83. PR #134 fixed #119, which removed the two Invoke-PfbNetworkPing / + # Invoke-PfbNetworkTrace `source.name` records (one of them a severity WRONG-RESULTS + # entry), and the committed report went 85 -> 83. Nothing red, because 85 is a ceiling and + # a DROP is the direction it calls better -- which is exactly why this needs re-lowering + # by hand: left at 85 the gate silently tolerates two brand-new dead keys, and would + # report safety it is no longer providing. + # + # Re-lower this whenever a fix drops the count. It stays a CEILING rather than becoming a + # pin: dead keys legitimately fall as fixes land, and a pin would red every such fix and + # make the gate a tax on doing the right thing. The cost of the ceiling is precisely the + # slack being closed here, so closing it promptly is the whole discipline. + $script:baselineDeadKeyCount = 83 $script:baselineNoSurvivingSelectorCount = 6 $script:baselineSkipReasons = @{ # New-PfbBucketAuditFilter|Name was introduced by 9d08ecc as a new parameter, so diff --git a/Tests/TestModuleImportGuard.Tests.ps1 b/Tests/TestModuleImportGuard.Tests.ps1 index 549c238..0d79e8e 100644 --- a/Tests/TestModuleImportGuard.Tests.ps1 +++ b/Tests/TestModuleImportGuard.Tests.ps1 @@ -2,7 +2,7 @@ <# UNGATED on edition, on purpose: pure AST plus file IO, 5.1-safe, no spec cache, and no graceful-skip path. It must contribute executed tests on both legs, which is why it - is listed in RequiredDescribes in BOTH blocks of coverage-baseline.psd1 -- MaxSkipped + is listed in RequiredDescribes in BOTH blocks of coverage-baseline.psd1 -- a skip count cannot see a Describe that vanishes without skipping. Deliberately does NOT try to detect a leaked shim. A test asserting "the module I can diff --git a/Tests/coverage-baseline.psd1 b/Tests/coverage-baseline.psd1 index 7f83c14..91b6173 100644 --- a/Tests/coverage-baseline.psd1 +++ b/Tests/coverage-baseline.psd1 @@ -5,26 +5,59 @@ # # Two independent assertions, because either alone has a gap: # - # MaxSkipped catches a Describe that starts reporting skips. + # ExpectedSkips an EXACT skipped-test count per test file. Catches a file that starts + # or stops reporting skips, and says which file. # RequiredDescribes catches a Describe that vanishes from the result tree entirely. # These blocks skip GRACEFULLY: when their guard evaluates false at # BeforeAll time, or the file is filtered out of a run, they contribute - # NEITHER a skip NOR a pass. A skip ceiling cannot see that, and a + # NEITHER a skip NOR a pass. A skip count cannot see that, and a # green summary is then indistinguishable from a silently-absent # assertion -- the same failure shape as #63, one level down. # - # MaxSkipped is a CEILING WITH HEADROOM, not a pin. Raise it deliberately in a reviewed - # diff when a change legitimately adds skipped tests, and say why in the commit message. + # ExpectedSkips REPLACED a single global MaxSkipped ceiling per edition (issue #132). The + # ceiling carried deliberate headroom, and that headroom was the defect rather than the + # mitigation. It did not prevent false reds; it converted an ATTRIBUTABLE red on the PR + # that moved the number into an UNATTRIBUTABLE red on a later, innocent PR, and it hid a + # real coverage regression of up to (ceiling - measured) tests -- the #63 failure shape, + # merely bounded in size. + # + # That is not hypothetical and the arithmetic was exact. A 5.1 run measured 292 against a + # ceiling of 268. Of the +40, six belonged to #120 two days EARLIER, which had slack and + # passed without touching this file; the next branch across the line was billed for all + # forty, including those six. The history of raises left in the winps51 block below is the + # audit trail of the same mechanism firing repeatedly. + # + # So these numbers are PINS, not ceilings. Any movement -- up or down -- reds the build, + # names the file, and is fixed by editing that file's line in the same diff that moved it. + # Do not reintroduce headroom, per-file or global; a per-file ceiling would be the same + # defect wearing a smaller hat. + # + # Three rails come with the map, in scripts/Assert-PfbTestCoverage.ps1: no test file may + # run EMPTY (contributing neither an executed nor a skipped test -- the #63 shape below + # the granularity RequiredDescribes can reach), no UNDECLARED file may skip, and a declared + # file that stops running at all is a violation rather than a stale entry to delete. + # + # Only files that actually skip appear here -- 19 of 199 on 5.1, 2 on pwsh 7 -- so this is + # a short list, not a per-file census. Attribution is by leaf file name, which is sound + # because Tests/ is flat and no two *.Tests.ps1 files share a leaf; the gate fails loudly + # if that ever stops being true. # # The two editions differ by design and by a wide margin: every Describe in the tooling # test files carries -Skip:($PSVersionTable.PSVersion.Major -lt 7), so the 5.1 leg skips - # all of them. A single shared ceiling would be either a permanent false red on 5.1 or + # all of them. A single shared map would be either a permanent false red on 5.1 or # useless on 7. pwsh7 = @{ - # Measured 2 on run 31359783827 (ubuntu-latest, pwsh, spec cache restored): 1842 - # passed / 0 failed / 2 skipped. 8 leaves room for a handful of legitimate additions - # without going so loose that a real regression hides under it. - MaxSkipped = 8 + # Seeded from run 32392093324 on main@98c7a16 (2026-08-20): 3328 passed / 0 failed / + # 2 skipped, and the same two files on all three pwsh legs -- ubuntu-latest, + # windows-latest and macos-latest agreed exactly, which is what makes one shared pwsh7 + # map correct rather than a windows-only measurement generalised. + # + # These two are ordinary per-test -Skip: guards on platform-specific behaviour, not + # the PS7 gating that dominates the winps51 block. Total: 2. + ExpectedSkips = @{ + 'New-PfbJwtToken.Tests.ps1' = 1 + 'Set-PfbTlsProtocol.Tests.ps1' = 1 + } RequiredDescribes = @( # Ungated -- these run on every leg and are the #63 regression guards themselves. 'Committed drift report (REGRESSION guard, no spec cache required)' @@ -94,6 +127,15 @@ ) } winps51 = @{ + # --------------------------------------------------------------------------------- + # RETAINED HISTORY of the MaxSkipped ceiling this block used to carry, kept verbatim + # because it is the evidence for issue #132 rather than a record of how to maintain + # anything. Read it as an audit trail: five deliberate raises, and by the last one the + # raise notes are visibly diagnosing the mechanism instead of the branches -- "268 was + # already stale against main before either branch existed". The numbers below are dead; + # nothing reads MaxSkipped any more. The live gate is the ExpectedSkips map after it. + # --------------------------------------------------------------------------------- + # # Re-measured 190 on run 31670025630 (windows-latest, Windows PowerShell 5.1): 1818 # passed / 0 failed / 190 skipped. The gap versus pwsh7's 2 is the PS7-gated tooling # Describes, skipping exactly as designed -- not a defect. @@ -161,7 +203,56 @@ # measurement. 313 keeps the standing +16 headroom over 297. If CI reports materially # more than 297, something other than these three files moved and the delta should be # re-attributed before raising again. - MaxSkipped = 313 + # + # --------------------------------------------------------------------------------- + # END OF RETAINED HISTORY. Live gate below. + # --------------------------------------------------------------------------------- + # + # Seeded from run 32392093324 on main@98c7a16 (2026-08-20, windows-latest, Windows + # PowerShell 5.1): 3033 passed / 0 failed / 297 skipped / 0 not-run, attributed across + # 19 files. The entries sum to exactly 297, which reconciles with the run's own total -- + # the gate asserts that reconciliation on every run, so a container the walk misses is + # a red rather than a quietly smaller number. + # + # Every entry is the same cause: a Describe carrying + # -Skip:($PSVersionTable.PSVersion.Major -lt 7), or a whole tooling file gated that way, + # because the generator or spec-walking code under test needs pwsh 7. They RUN on 7 -- + # that leg skips 2 in total -- so these are the PS7 gate working as designed, not lost + # coverage. That is why the two editions need separate maps. + # + # When one of these numbers moves, the gate names the file and the delta. Update the + # line and say why in the commit message. Do NOT pad it. + ExpectedSkips = @{ + # Tooling files gated wholesale: no executed tests on 5.1 at all. These are the + # ones the empty-container rail would flag if their -Skip: guards were ever + # replaced with `#Requires -Version 7.0`, which would take them out of the skip + # count entirely and make them invisible here. + 'Build-PfbApiDriftReport.Tests.ps1' = 65 + 'Build-PfbCapabilityMap.Tests.ps1' = 50 + 'Update-PfbTestModuleImport.Tests.ps1' = 34 + 'PfbPipelineSelectorTools.Tests.ps1' = 33 + 'PfbSelectorProbeHarness.Tests.ps1' = 16 + 'Build-PfbFieldCmdletMap.Tests.ps1' = 15 + 'PfbPipelineSelectorRail.Tests.ps1' = 11 + 'Build-PfbResponseShapeMap.Tests.ps1' = 9 + 'Build-PfbDeadKeyReport.Tests.ps1' = 6 + # Mixed files: some Describes PS7-gated, others deliberately ungated so they + # execute on both legs. The passing halves are what several RequiredDescribes + # entries above are asserting, so these two numbers moving in opposite directions + # is a real signal rather than noise. + 'Build-PfbValueEnumMap.Tests.ps1' = 14 + 'Build-PfbCapabilityMap.ContextScopeDrift.Tests.ps1' = 13 + 'PfbApiDriftTools.Tests.ps1' = 8 + 'PfbValueEnumTools.Tests.ps1' = 8 + 'PfbSpecTools.Tests.ps1' = 6 + 'PfbSpecTools.ContextScope.Tests.ps1' = 4 + 'Issue31.DriftConfidence.Tests.ps1' = 1 + # Not PS7 gating: ordinary per-test guards on platform or edition behaviour. The + # first two also appear in the pwsh7 map, at lower counts. + 'New-PfbJwtToken.Tests.ps1' = 2 + 'Set-PfbTlsProtocol.Tests.ps1' = 1 + 'Connect-PfbArray.OAuth2TokenRefresh.Tests.ps1' = 1 + } # Only the ungated blocks are required here. The six spec-cache blocks above are # PS7-gated by design, so requiring them on 5.1 would be a permanent false red. RequiredDescribes = @( diff --git a/scripts/Assert-PfbTestCoverage.ps1 b/scripts/Assert-PfbTestCoverage.ps1 index 4e6d48c..3e9cd2d 100644 --- a/scripts/Assert-PfbTestCoverage.ps1 +++ b/scripts/Assert-PfbTestCoverage.ps1 @@ -9,12 +9,19 @@ Two independent assertions, because either alone has a gap: - 1. Skip ceiling -- catches a Describe that starts reporting skips. - 2. Required-Describe allowlist -- catches a Describe that vanishes from the result tree - entirely. These blocks skip GRACEFULLY: when their guard evaluates false at BeforeAll - time, or the file is filtered out of a run, they contribute NEITHER a skip NOR a pass. - A skip ceiling cannot see that, and a green summary is then indistinguishable from a - silently-absent assertion -- the same failure shape as #63, one level down. + 1. Required-Describe allowlist -- catches a NAMED Describe that vanishes from the result + tree entirely. These blocks skip GRACEFULLY: when their guard evaluates false at + BeforeAll time, or the file is filtered out of a run, they contribute NEITHER a skip + NOR a pass. A skip count cannot see that, and a green summary is then + indistinguishable from a silently-absent assertion -- the same failure shape as #63, + one level down. + 2. Per-file skip attribution (issue #132) -- an EXACT expected skip count per test file, + plus three rails around it: no file may run empty, no undeclared file may skip, and + a declared file that stops running at all is a violation rather than a tidy-up. + + Assertion 2 replaced a single global MaxSkipped ceiling per edition. The ceiling's headroom + turned out to be the defect and not the mitigation -- see the long note at assertion 2 and + issue #132 -- so the numbers here are pinned, and movement is attributed to a file. Takes a result OBJECT rather than running Pester itself, so it is testable with a hand-built stand-in and needs neither a real suite run nor the spec cache. @@ -28,6 +35,12 @@ Result shape verified against Pester 6.0.0 and 6.0.1 on both editions: $Result.Tests is a flat list, each entry has .Path (a List whose [0] is the top-level Describe name) and a .Result string; the run carries PassedCount/FailedCount/SkippedCount/NotRunCount. + $Result.Containers is one entry per test FILE, each with .Item (a FileInfo, so .FullName) + and its own PassedCount/FailedCount/SkippedCount/NotRunCount. + + The two collections disagree in a way assertion 2 depends on: a file killed by `#requires` + contributes NOTHING to .Tests but still appears in .Containers with all four counts zero. + Probed directly on both editions rather than assumed. .PARAMETER Result The object returned by Invoke-Pester -PassThru. .PARAMETER Edition @@ -83,17 +96,118 @@ foreach ($required in @($editionBaseline.RequiredDescribes)) { $summaryLines.Add("| ``$required`` | $ran ($marker) | $skipped |") } -# --- Assertion 2: skip ceiling ------------------------------------------------------------- -$skippedTotal = [int]$Result.SkippedCount + [int]$Result.NotRunCount -$ceiling = [int]$editionBaseline.MaxSkipped +# --- Assertion 2: per-file skip attribution (issue #132) ----------------------------------- +# Replaces the global MaxSkipped ceiling. That ceiling carried deliberate headroom, and the +# headroom was the defect rather than the mitigation: it silently absorbed skip movement until +# some later, unrelated PR crossed the line, and then billed that PR for the whole accumulated +# delta with no signal saying which part was its own. It happened exactly once and the +# arithmetic was exact -- a 5.1 run measured 292 against a ceiling of 268, and 6 of the +40 +# belonged to #120 two days earlier, which had passed on slack. See issue #132. +# +# Per-file numbers are EXACT, not ceilings. Any movement -- up or down -- reds the PR that +# caused it and names the file, which is the whole point. A file that legitimately gains or +# loses skipped tests updates its own line in the same diff. +# +# Attribution comes from $Result.Containers, NOT from $Result.Tests, and the difference is +# load-bearing: a file whose `#requires` stops it before discovery contributes NO entries to +# .Tests at all, but still appears in .Containers with every count at zero. That is the +# issue-#63 shape, and .Containers is the only place it is visible. Verified on Pester 6.0.1 +# under pwsh 7 and 6.0.0 under Windows PowerShell 5.1. +$expectedSkips = $editionBaseline.ExpectedSkips +if ($null -eq $expectedSkips) { + throw "Coverage baseline block '$Edition' has no ExpectedSkips map. Issue #132 replaced MaxSkipped with a per-file map; a block carrying neither gates nothing." +} + +$containers = @($Result.Containers) + +# ANTI-VACUITY, before anything reads the walk. Without this, an empty or absent .Containers +# makes every 'declared file is missing' check fire at once -- a wall of confusing reds whose +# real cause is that the walk found nothing. Fail once, saying so. +if ($containers.Count -lt 1) { + $violations.Add("The Pester result carries no Containers, so per-file skip attribution could not run at all. Every assertion below would be meaningless. Check the Pester version and that Run.PassThru was set.") +} + +# Flatten to leaf file name -> counts. Tests/ is flat and no two *.Tests.ps1 files share a +# leaf name (checked: 199 files, 0 duplicates), so the leaf is a safe key and keeps the +# baseline free of runner-specific absolute paths. +$observed = @{} +$attributedSkips = 0 +foreach ($container in $containers) { + $name = '' + if ($container.Item) { + if ($container.Item.FullName) { $name = Split-Path -Leaf $container.Item.FullName } + else { $name = Split-Path -Leaf ([string]$container.Item) } + } + if (-not $name) { continue } + + $executed = [int]$container.PassedCount + [int]$container.FailedCount + $skipped = [int]$container.SkippedCount + [int]$container.NotRunCount + $attributedSkips = $attributedSkips + $skipped + + if ($observed.ContainsKey($name)) { + # Same leaf twice means the leaf-name key is no longer unique, which would let one + # file's numbers mask another's. Say so rather than silently overwriting. + $violations.Add("Two containers share the leaf name '$name', so per-file attribution by leaf name is no longer sound. Key the baseline by repo-relative path instead.") + continue + } + $observed[$name] = @{ Executed = $executed; Skipped = $skipped } +} +# --- 2a: no container ran empty ------------------------------------------------------------ +# The issue-#63 shape, one level below RequiredDescribes. That allowlist catches a named +# Describe vanishing, but only for the blocks someone remembered to list. This catches ANY +# test file that contributed neither an executed nor a skipped test -- a throwing top-level +# BeforeAll, or a `#requires` the runner does not satisfy. Green and non-vacuous today: all +# 199 files contribute something on all four CI legs. +foreach ($name in ($observed.Keys | Sort-Object)) { + if ($observed[$name].Executed -lt 1 -and $observed[$name].Skipped -lt 1) { + $violations.Add("Test file '$name' contributed no tests at all -- neither executed nor skipped. Its BeforeAll threw, or a #requires stopped it before discovery. This is the issue-#63 shape: the run stays green while the file's assertions silently do not exist.") + } +} + +# --- 2b: declared files match exactly, and are still present ------------------------------- $summaryLines.Add('') -$summaryLines.Add("Skipped + not-run: **$skippedTotal** (ceiling $ceiling). Passed: $($Result.PassedCount). Failed: $($Result.FailedCount).") +$summaryLines.Add('| Test file | Skipped | Expected | |') +$summaryLines.Add('|---|---:|---:|---|') + +foreach ($name in (@($expectedSkips.Keys) | Sort-Object)) { + $expected = [int]$expectedSkips[$name] + if (-not $observed.ContainsKey($name)) { + $violations.Add("ExpectedSkips declares '$name' = $expected, but no such test file ran. Either the file was renamed or deleted and this entry is stale, or it dropped out of discovery -- which is lost coverage, not a tidy-up.") + $summaryLines.Add("| ``$name`` | -- | $expected | ABSENT |") + continue + } + $actual = $observed[$name].Skipped + $marker = 'OK' + if ($actual -ne $expected) { + $marker = 'MOVED' + $delta = $actual - $expected + $sign = '+' + if ($delta -lt 0) { $sign = '' } + $violations.Add("'$name' skipped $actual tests, expected exactly $expected ($sign$delta). If that movement is correct -- a PS7-gated Describe added or removed -- update this file's entry in Tests/coverage-baseline.psd1 in the same diff. If it is not, coverage moved without anyone deciding to move it.") + } + $summaryLines.Add("| ``$name`` | $actual | $expected | $marker |") +} + +# --- 2c: nothing skips that has not declared it -------------------------------------------- +foreach ($name in ($observed.Keys | Sort-Object)) { + if ($observed[$name].Skipped -lt 1) { continue } + if ($expectedSkips.ContainsKey($name)) { continue } + $violations.Add("'$name' skipped $($observed[$name].Skipped) tests but is not declared in the $Edition ExpectedSkips map. A new file that skips must say so, and say why in the comment beside it.") + $summaryLines.Add("| ``$name`` | $($observed[$name].Skipped) | (undeclared) | UNDECLARED |") +} -if ($skippedTotal -gt $ceiling) { - $violations.Add("Skipped + not-run count $skippedTotal exceeds the $Edition ceiling of $ceiling. Either coverage regressed, or the ceiling in Tests/coverage-baseline.psd1 needs a deliberate, reviewed raise.") +# --- 2d: the walk saw every skip the run reported ------------------------------------------ +# Reconciliation, not a coverage assertion. If per-container skips do not sum to the run's own +# total, the walk missed a container and 2a-2c were all judging an incomplete picture. +$skippedTotal = [int]$Result.SkippedCount + [int]$Result.NotRunCount +if ($containers.Count -ge 1 -and $attributedSkips -ne $skippedTotal) { + $violations.Add("Per-file attribution accounts for $attributedSkips skipped/not-run tests, but the run reports $skippedTotal. The container walk is incomplete, so every per-file verdict above is judging a partial picture.") } +$summaryLines.Add('') +$summaryLines.Add("Skipped + not-run: **$skippedTotal**, attributed across $($observed.Count) files. Passed: $($Result.PassedCount). Failed: $($Result.FailedCount).") + if ($env:GITHUB_STEP_SUMMARY) { ($summaryLines -join [Environment]::NewLine) | Out-File -FilePath $env:GITHUB_STEP_SUMMARY -Append }