fix(coverage): exclude nested lambdas of exempt members from the denominator - #542
Merged
drmoisan merged 2 commits intoAug 11, 2026
Conversation
…minator A method-level [ExcludeFromCodeCoverage] does not suppress lambdas declared inside the attributed member: the compiler hoists them into a closure type that does not inherit the attribute, so their lines stay in the Cobertura denominator. Files adopting the repository's thin exempt production forwarder seam therefore carried a permanent, invisible coverage ceiling. Add scripts/vscode/Invoke-MSTestWithCoverage.ClosureFilter.ps1, a pure XML-to-XML transform that drops closure-class coverage whose declaring member is absent from the package's instrumented-member presence set. The presence set admits Type.<Member>d__<N> state-machine class names, so lambdas inside non-exempt async members are retained. An unrecognized compiler-generated name shape causes retention, never removal: over-exclusion is not an acceptable failure mode. Wire it into ConvertTo-KoverageCoberturaXml after path normalization and before Merge-CoberturaClassesByFilename. The ordering is a constraint, not a preference: a closure type shares its declaring type's filename, so the merge collapses it into a node named for the declaring type that carries neither the .<>c marker nor the <Member>b__ methods the filter resolves against. Placed after the merge the filter is a no-op, which regression case 6 pins end to end. Measured against the post-#441 arithmetic, repository lines-valid falls 62873 -> 62401 and the line rate rises 85.3514% -> 85.5355%. QuickFiler/Viewers/BreadcrumbPopupUiOperations.cs goes 90.70% -> 99.15%, and TaskVisualization/FlagTasks.cs leaves the report entirely because every member of the type is attributed. No coverage threshold is changed; threshold reconciliation is owned by issue #494. Three residuals are documented and handed off as potential entries rather than absorbed: lambdas inside exempt async members, local functions, and overload-name collisions. Closes #457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dings Feature review returned zero blocking findings. The reviewer verified the ordering constraint substantively rather than from evidence prose: applying merge-then-filter in a scratch probe leaves the exempt line in place and makes regression case 6 fail, confirming the test genuinely pins the constraint rather than passing vacuously. Promote the two non-blocking code-review findings to potential entries so they survive the merge rather than living only in a feature-folder artifact: - CR-1: the filter's retained-line rebuild duplicates the merge's line-map loop while omitting stale condition-coverage removal and <conditions> copying. The duplication was forced -- the helpers module sat at 455 of 500 lines and spec AC 13 fixed its change surface at exactly two added lines, so extracting a shared helper was unavailable to #457. - CR-3: SupportsShouldProcess on a pure in-memory transform means a session $WhatIfPreference of $true skips filtering silently, emitting a plausible but unfiltered denominator. The attribute was adopted because PSScriptAnalyzer raises PSUseShouldProcessForStateChangingFunctions against a bare Remove- verb and the analyze gate exits non-zero on a warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A method-level
[ExcludeFromCodeCoverage]does not suppress lambdas declared inside the attributed member. The compiler hoists each lambda into a closure type —Ns.T.<>c,Ns.T.<>c__DisplayClass41_0— that does not inherit the attribute, so the collector instruments it and its lines stay in the Cobertura denominator. Any file that adopts this repository's preferred "thin exempt production forwarder" seam therefore carries a permanent coverage ceiling that no amount of testing can lift, and the ceiling is invisible in the report.This is the second half of the correctness problem that #441 opened. #441 governs how lines are counted; #457 governs which lines enter the denominator at all. The acceptance evidence below is a corrected per-file rate, which only became measurable once #441 landed.
Closes #457.
What changed
A new production module,
scripts/vscode/Invoke-MSTestWithCoverage.ClosureFilter.ps1, implements a pure XML-to-XML transform. For each<package>it builds a presence set of instrumented members, then for each closure class derives each method's declaring member and drops the methods whose declaring member is absent from that set. A class is removed outright only when at least one method was dropped and zero methods are retained.scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1receives exactly two added lines and zero removed: a dot-source, and a singleRemove-CoberturaExemptClosureCoverage -XmlDocument $xmlcall.Three invariants worth reviewing closely
Call-site ordering is a correctness constraint, not a preference. The filter runs after the
//class[@filename]normalization loop and beforeMerge-CoberturaClassesByFilename. A closure type always shares its declaring type'sfilename, so the merge collapses it into a node named for the declaring type — a node that carries neither the.<>cmarker nor the<Member>b__…method names the filter resolves against. Placed after the merge, the filter is not merely less effective; it is a silent no-op. Regression case 6 drives a fixture end-to-end to pin this. The reviewer independently confirmed the test is not vacuous by applying merge-then-filter in a scratch probe: the exempt line survives and case 6's assertions fail.Retention is fail-safe. A method whose declaring member cannot be derived is retained, never dropped. Over-exclusion would silently shrink the denominator and inflate the rate, which is precisely the class of error this epic exists to eliminate. Regression case 4 exercises this at the orchestrator level with a
.ctoron a bare<>cclass, whose declaring member is derivable from neither the method name nor the class name.Lambdas inside non-exempt
asyncmembers are retained. The presence set admitsType.<Member>d__<N>state-machine class names as proof that a declaring member was instrumented. Without that source, a lambda inside an ordinary async member would be wrongly deleted. The admitting regex is end-anchored so that nested<<M>b__0>dclasses cannot admit spurious members.Measured results
Repository-wide, both sides captured against the post-#441 arithmetic:
lines-coveredlines-validline-ratebranches-coveredbranches-validbranch-ratePer-file:
QuickFiler/Viewers/BreadcrumbPopupUiOperations.cs: line rate 90.70% → 99.15%,<line>count 258 → 234.TaskVisualization/FlagTasks.cs: present → absent. Every member of the type is attributed, so the file correctly leaves the report entirely.These figures are measured, not derived. The corrected rate is not
covered / (valid - 22):<>c__DisplayClass42_0contributes two covered lines from the exempt memberDisposeProductionSurface, which the filter removes from both the numerator and the denominator.No coverage threshold is changed by this PR. The post-change repository line rate is 85.5355%, above the 85% floor. Threshold reconciliation is owned by #494 in epic wave 2, which is blocked on this feature precisely so it can decide against a corrected figure.
Verification
PowerShell toolchain, converged on iteration 2 (
run_poshqc_format→run_poshqc_analyze→run_poshqc_test):Invoke-MSTestWithCoverage.Helpers.ps1: 90.69% (185/204) against a 90.59% baseline; both added lines covered and the missed-line set unchanged from baseline.Invoke-ScriptAnalyzer: exactly one diagnostic, the pre-existingPSUseSingularNounsonGet-CoberturaLineConditionCoverageParts. Renaming it would exceed the two edits fixed by AC 13.All 16 acceptance criteria in
spec.mdare checked off. Feature review returned zero blocking findings across the policy audit, code review, and feature audit.Ten regression cases run as individually named Pester tests. Every fixture is an inline here-string: no temporary files, no on-disk fixtures, no committed
.cssources.Known conditions this PR does not address
InitializeBool_ThroughThePumpHost_CompletesAndInitializesStateandInitializeNineArgOverload_ThroughThePumpHost_SavesParametersAndDelegates, with counts identical before and after (6435 / 6433 / 2). Already tracked as Bug: winformspumphost-tests-load-flaky-visible-window #511, which names these cases explicitly. This PR is PowerShell-only.artifacts/pester/powershell-coverage.xmlreports zero covered lines repository-wide — the known bundled-tool defect tracked as Bug: poshqc-test-coverage-capture-records-zero #536. Every PowerShell figure above comes from a paired direct-Pester run.Follow-ups opened rather than absorbed
Five potential entries, so that none of this survives only as prose in a feature folder:
Three documented residuals — lambdas inside exempt async members remain counted (admitting
d__classes is mandatory for the async guard above); local functions (<Member>g__Local|N_M) remain counted; overload-name collisions cause under-exclusion, never over-exclusion.Two review findings — the retained-line rebuild duplicates the merge's line-map loop while omitting stale
condition-coverageremoval and<conditions>copying; andSupportsShouldProcesson a pure in-memory transform means a session$WhatIfPreferenceof$trueskips filtering silently. Both were forced by constraints this feature could not relax: the helpers module sat at 455 of 500 lines with its change surface fixed at two edits, and PSScriptAnalyzer raisesPSUseShouldProcessForStateChangingFunctionsagainst a bareRemove-verb while the analyze gate exits non-zero on a warning.Base branch
This PR targets
epic/build-ci-coverage-gate-fidelity-integration, notmain..github/workflows/ci.ymltriggerspull_requestonly onmainanddevelopment, so no workflow runs against this PR by design. The integrated tree is gated separately.🤖 Generated with Claude Code