Skip to content

Enforce the C# coverage floors in CI and add a Pester gate for scripts/vscode - #897

Merged
drmoisan merged 17 commits into
mainfrom
bug/ci-coverage-threshold-and-pester-gates-869
Sep 14, 2026
Merged

drmoisan merged 17 commits into
mainfrom
bug/ci-coverage-threshold-and-pester-gates-869

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Enforce the C# coverage floors in CI and add a Pester gate for scripts/vscode

Summary

  • Adds a C# branch-coverage assertion at 75 percent and wires it into the coverage entry point immediately after the existing line assertion, so both floors are evaluated on every run.
  • Rewrites the MSTest coverage workflow to run scripts/vscode/Invoke-MSTestWithCoverage.ps1 instead of an inline vstest.console.exe block, which is what makes the pre-existing 80 percent line assertion actually execute in CI.
  • Adds .github/workflows/_pester.yml, a new reusable callee that runs the tests/scripts/vscode suite with JaCoCo coverage and fails the job below an 80 percent LINE figure or on any test failure.
  • Guards the top-level bodies of Invoke-VSBuild.ps1 and Invoke-Restore.ps1 behind extracted main functions, so dot-sourcing them in a test no longer builds the solution or rewrites project files.
  • Raises scripts/vscode line coverage from 78.90 percent to 83.93 percent, clearing the floor the new gate enforces.
  • One acceptance criterion is intentionally left unchecked: the live check-run context name cannot be captured until a pull request run exists.

Why

Two coverage gates were specified but not enforced. The MSTest workflow produced a Cobertura document through an inline vstest invocation that never reached the threshold assertion, so the 80 percent line floor existed in code and was never evaluated by CI. No PowerShell coverage gate existed at all, despite scripts/vscode carrying the repository's build, restore, test and coverage tooling.

A second, narrower defect blocked the PowerShell gate from being added safely. Invoke-VSBuild.ps1 and Invoke-Restore.ps1 executed their work at file scope, so any Pester file that dot-sourced them ran a real build and, through the package-reference sync script, rewrote .csproj files in the working tree. Coverage of those scripts was therefore both unmeasurable and destructive to measure. Extracting the bodies behind an invocation guard is what makes the measurement deterministic, which the determinism proof in this change demonstrates against the pre-change per-file counters.

What Changed

CI workflows

  • .github/workflows/_mstest-coverage.yml — replaces the inline vstest block with a single pwsh step invoking the coverage entry point; pins the dotnet-coverage install version; repoints the artifact upload at coverage/coverage.cobertura.xml and changes the no-files behaviour from warn to error.
  • .github/workflows/_pester.yml — new reusable callee: windows-latest, 10-minute job timeout, contents: read, both workflow_call and workflow_dispatch, no concurrency block, pinned Pester 5.x install and import, artifact upload with if-no-files-found: error.
  • .github/workflows/ci.yml — adds the pester caller job in the existing leading-dot relative form.
  • .github/workflows/README.md — updates the gate table, records both pinned tool versions, and adds the new required check-run context. The prior claim that the vstest invocation was moved rather than edited is withdrawn, because this change deliberately replaces it and thereby changes the gate's pass criterion.

Production PowerShell

  • scripts/vscode/Invoke-MSTestWithCoverage.Threshold.ps1 — adds Assert-CoberturaBranchCoverageThreshold, with distinct terminating messages for a missing rate, a non-numeric rate, a rate outside the closed unit interval, zero valid branches, and a below-floor rate.
  • scripts/vscode/Invoke-MSTestWithCoverage.ps1 — one statement, calling the branch assertion on the same post-processed document, immediately after the line assertion and before the first-party report.
  • scripts/vscode/Invoke-VSBuild.ps1 — body extracted into Invoke-VSBuildMain behind an invocation guard; three wrapper seams (Get-MSBuildPath, Invoke-SyncPackageReferences, Invoke-MSBuildExe). Guard order, guard messages and the argument list are unchanged.
  • scripts/vscode/Invoke-Restore.ps1 — the same pattern via Invoke-RestoreMain, with deliberately distinct seam names (Get-RestoreMSBuildPath, Invoke-RestoreMSBuildExe) because Pester runs every container in one runspace and same-named functions dot-sourced from two files would shadow each other.

Tests

New: Invoke-Restore.Tests.ps1, TestProcessCleanup.Tests.ps1. Extended: the threshold, assembly-discovery, merge, SDK-install, main and VSBuild suites. Repaired: three files whose mocked post-processed Cobertura document carried no branch-rate and which the new assertion would otherwise turn red.

Documentation and evidence

Feature documents and the full per-phase evidence tree, including fail-before artifacts for every regression test and negative-path proofs for both gates.

Architecture / How It Fits Together

ci.yml calls both reusable callees. The MSTest callee runs the coverage entry point, which collects via dotnet-coverage, post-processes the Cobertura document, then evaluates the line assertion and the new branch assertion against the document root before emitting the first-party report and the JaCoCo projection. The Pester callee builds a Pester configuration over tests/scripts/vscode with coverage scoped to scripts/vscode, writes JaCoCo to an explicit path, prints the counts and the measured figure, and only then evaluates its exit condition — the explicit exit is placed after the figure-emitting statements so a failure is always diagnosable from the log.

Note that the C# gate is judged on the document-root attributes, which are computed over every package surviving third-party removal. The First-party coverage: line the run prints is computed over the first-party allowlist and is a different denominator. Both are recorded, labelled, and compared only like with like.

Verification

Completed in this change. All figures below come from the final toolchain pass, which completed cleanly in a single pass.

Gate Threshold Result
PowerShell LINE, scripts/vscode 80 percent 83.93 percent (731/871)
New PowerShell code 90 percent 98.36 percent (60/61)
C# line, document root 80 percent 85.88 percent
C# branch, document root 75 percent 80.05 percent, 17022 valid branches
  • Pester 5.6.1: 174 passed, 0 failed, 0 skipped.
  • PSScriptAnalyzer: 16 diagnostics, equal to the recorded baseline; the four diagnostics naming a write-set path are byte-identical by rule and file to the baseline rows.
  • PoshQC formatter: rewrote no file, established from before-and-after porcelain rather than from the exit code.
  • actionlint: exit 0 across the eight workflow files now present.
  • CSharpier check: 1639 files, unformatted list identical to the baseline list. No C# source file is modified by this change.
  • Both gates were additionally proven to fail closed: a below-floor branch projection and a zero-valid-branch projection each produce a non-zero exit, and the Pester gate goes red both on reduced coverage and on a failing test case.

Recommended for the reviewer.

pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-MSTestWithCoverage.ps1 -SearchRoot . -Configuration Debug
pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/dev-tools/run-actionlint.ps1

Backward Compatibility / Migration Notes

  • No public API changes. No C# source, project, properties or targets file is modified.
  • The MSTest coverage job's pass criterion changes: it can now fail on coverage where previously it could not.
  • CI adopts the runsettings the local script passes, which declares class-level test scope with one worker per core. This is accepted for the first run; any new failure should be treated as a finding rather than a flake.
  • The trx logger is no longer passed, because the script's argument builder does not pass it. The artifact upload is repointed at the Cobertura document accordingly.
  • Requires a maintainer action outside this change: the branch ruleset must add the one new required context. The change adds exactly one new context, not two — the C# branch assertion changes no job name.

Risks and Mitigations

  • A new gate can turn CI red on unrelated work. Both floors were measured against the current tree with margin before being enforced, and the delivery raises the PowerShell figure well past the floor rather than setting the floor to fit the tree.
  • Class-level parallelization is new to CI. Recorded explicitly as an accepted behaviour change with the two runsettings values it depends on quoted.
  • Rollback is a straight revert. The gates are additive; reverting restores the prior non-enforcing behaviour without touching production C#.

Review Guide

  1. scripts/vscode/Invoke-MSTestWithCoverage.Threshold.ps1 and the single call site — the smallest and most load-bearing diff.
  2. .github/workflows/_pester.yml, particularly the ordering of the exit statement relative to the figure-emitting statements.
  3. scripts/vscode/Invoke-VSBuild.ps1 and Invoke-Restore.ps1 — the largest production diffs, but mechanical extraction; guard order and messages are unchanged.
  4. Test files — largest by line count and the noisiest part of the diff.
  5. The evidence tree, if you want the measured basis for any figure quoted above.

Follow-ups

  • The live check-run context name is not yet captured, because no run exists against the head SHA until this pull request opens. One acceptance criterion remains unchecked for that reason, and the evidence artifact records the exact query to re-run.
  • A pre-existing analyzer reference skew was found while building this branch and is not addressed here, because every affected file is a .csproj and therefore outside this change's declared write set. Fifteen project files carry an unconditional analyzer item naming Meziantou.Analyzer.3.0.203 while the restore manifests pin 3.0.235, so a clean clone fails to build. CI does not currently surface this, because the NuGet cache step uses a prefix fallback key that carries the older package forward. It warrants its own issue.

GitHub Auto-close

🤖 Generated with Claude Code

https://claude.ai/code/session_01Mok2zkoP23ArYqAjHvRqd1

drmoisan and others added 17 commits September 12, 2026 13:09
…eflight-cleared plan

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…fied plan premises

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d tree

Round 1 repaired the five premises item 873 invalidated: re-based the eleven P0-T4 baseline line counts (Invoke-MSTestWithCoverage.ps1 351->438, Invoke-MSTest.RunSettings.Tests.ps1 496->498, AssemblyDiscovery.Tests.ps1 99->106, Invoke-MSTest.Main.Tests.ps1 144->146); retargeted P2-T3 onto the enriched fixture at line 55, adding the branch attributes rather than deleting the per-line data 873 depends on; retargeted P2-T4 onto sites 382, 416 and 418 and corrected its unsatisfiable 496-line acceptance to 498; corrected P10-T10's stale pre-change figure; and replaced P0-T7's single halt branch with five disjoint branches so a projection or reconciliation failure below the report statement is no longer misattributed to a below-floor coverage figure. Round 2 repaired three further defects measured afterwards: all 46 occurrences of the bare-dot git form, which resolved to the coordinator session worktree rather than this item's, are converted and a working-directory rule added to the preamble; the two distinct coverage denominators are now labelled and never compared across; and the tool manifest location, the Pester version-qualified import and the coverage ignore rule are recorded. Structure preserved at 11 phases and 114 tasks. The MCP plan validator returns ok with no warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… atomic plan

Executor preflight returned REVISIONS REQUIRED with six defects. D1 was blocking: run-actionlint.ps1 line 11 invokes the binary with no path argument, so an absolute -File path fixed only the binary lookup while actionlint still resolved its lint target from the working directory, which is the coordinator session worktree. All three actionlint spans would have linted the wrong tree, and because both trees hold the same seven workflow files the baseline would have passed silently, leaving the post-change gates unable to observe _pester.yml or the edits to ci.yml. D2 removed assertions over file counts and path names that actionlint never prints, replacing them with a Glob listing taken in the item worktree. D3 replaced an unreachable batch-budget halt branch, since the hook derives its state root from PSScriptRoot and allows out-of-root candidates, so no denial can occur. D4 restated a justification that described a superseded document state. D5 gave the AC-02 agreement predicate a mechanical string-equality definition across the two coverage denominators. D6 named the batch-1 formatter step in the commit exoneration clause. Two further defects were then measured and repaired: the dotnet-coverage version source was inverted, because the tool prints build metadata that NuGet cannot resolve and the workflow pin required byte-identity; and the repo-local .NET SDK was added as an explicit prerequisite, since global.json pins 8.0.205 and no worktree carried it. Structure preserved at 11 phases and 114 tasks. The MCP plan validator returns ok with no warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dual plan citations

The confirming preflight verified that all eight earlier repairs landed without breaking any command quoting, then reported six residual defects. E1 is the substantive one: the solution build depends on Meziantou.Analyzer 3.0.203, which fifteen project files reference through an unconditional Analyzer item while every packages.config that names the package pins 3.0.235, so nuget restore can never produce it and a fresh clone fails with CS0006. The same divergence is present on origin/main, so it is pre-existing and outside this delivery's scope; P0-T6 now records the prerequisite, checks for the assembly before building, and forbids repairing it by editing a project file, since every project file sits outside the declared write set. E2 through E6 correct a standing-rule enumeration that a later round invalidated, a wrong line count, a mis-cited render line at three sites, ordinals that a new prerequisite made ambiguous, and a detached qualifier. A final round corrected a claim that altcover had been installed: that package is absent from packages.config and reachable only through Exists-guarded imports, so installing it would activate imports intended to stay dormant. It was removed and a full rebuild confirms it is not load-bearing. Structure preserved at 11 phases and 114 tasks; the MCP plan validator returns ok with no warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng assertion site

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…main functions

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e and SDK install targets

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-uplift coverage basis

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e survives strict mode

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ance criteria

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lan checklist

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The executor modified tests/scripts/vscode/Invoke-MSTestWithCoverage.ResultsDirectory.Tests.ps1 and escalated it rather than absorbing it silently, because the path was absent from the declared write set in the plan and the specification. The file arrived with the merged evidence-projection item after this write set was authored, and carries the same mocked post-processed Cobertura document with no branch-rate that P2-T3 and P2-T4 repair in two sibling files, which the branch assertion added by P2-T1 and wired into the entry point by P2-T2 turns red. The same minimal in-place repair was applied, adding branch-rate and branches-valid and leaving the file at 268 lines. Reverting was not an option, since it would leave failing tests in the delivered suite, so the documents were the defect rather than the change. Both write sets now name the path and record its provenance, which restores P10-T14's write-set acceptance to true. No task, acceptance condition or checkbox state was altered; the plan holds 114 checked tasks and 11 phases and the MCP plan validator returns ok.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mok2zkoP23ArYqAjHvRqd1
@drmoisan
drmoisan merged commit 91746d2 into main Sep 14, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant