Uh oh!
There was an error while loading. Please reload this page.
fix: Report real code coverage percentages in Test-PSBuildPester - #165
Open
tablackburn wants to merge 2 commits into
Open
fix: Report real code coverage percentages in Test-PSBuildPester#165tablackburn wants to merge 2 commits into
tablackburn wants to merge 2 commits into
Conversation
Each JaCoCo counter percentage was computed as
[Math]::Truncate([int]$_.covered / $total)
and [Math]::Truncate collapses any fraction to 0, so the value was 0 for
every coverage level below exactly 100%. Two consequences fell out of the
same number: the printed coverage report always read 0.00% (or 100.00%),
and the CodeCoverageThreshold comparison failed the build for any nonzero
threshold unless coverage was exactly 100% -- coverage gating could not be
used at all.
The ratio is now kept as a fraction between 0 and 1, matching what the
{2:p} format string and the documented ".90 = 90%" threshold expect. It is
deliberately not rounded, so a threshold is never met by a value that only
rounds up to it.
The change is strictly more permissive: a build that passed with a coverage
threshold set still passes.
Tests bracket the reported value from both sides against a fixture with
partial coverage -- a threshold below it must pass, one above it must fail
-- which pins the value to a fraction and catches both a truncated 0 and a
0-to-100 scale. The subprocess helper now captures the command's output
instead of letting the report lines interleave with the job result object.
Closes#138
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TJfFJGtUJY5CFu8MRRMYtThere was a problem hiding this comment.
Pull request overview
Fixes Test-PSBuildPester coverage calculations so JaCoCo percentages and thresholds work correctly.
Changes:
- Preserves fractional coverage ratios.
- Adds threshold regression tests.
- Documents the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Summary |
|---|---|
tests/Test-PSBuildPester.tests.ps1 | Adds coverage threshold tests and captures subprocess output. |
PowerShellBuild/Public/Test-PSBuildPester.ps1 | Corrects fractional coverage calculation. |
CHANGELOG.md | Documents the coverage fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results 4 files 696 suites 5m 0s ⏱️ Results for commit 7817e5a. ♻️ This comment has been updated with latest results. |
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 freeto 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
Test-PSBuildPestercomputed every JaCoCo counter percentage as[Math]::Truncate([int]$_.covered / $total).[Math]::Truncatecollapses any fraction to0, so the value was0for every coverage level below exactly 100%.One wrong number, two user-facing consequences:
0.00%(or100.00%)CodeCoverageThresholdcomparison failed the build for any nonzero thresholdunless coverage was exactly 100%, so
$PSBPreference.Test.CodeCoverage.Thresholdcouldnot be used at all
The ratio is now kept as a fraction between 0 and 1, which is what the
{2:p}format stringand the documented
.90 = 90%threshold already expected.Not rounded, on purpose.#138 offered rounding as an alternative. Rounding
0.7996up to0.80would let a threshold of0.80be met by coverage that does not actually meet it, anda coverage gate that reports "met" when it isn't is the same class of bug being fixed here.
Test Plan
Two tests bracket the reported value from both sides, against the existing coverage fixture
(the scenario exercises only
Get-Widgetwhile measuring both public fixture functions, socoverage is always partial):
0.010— this is the failing test for #1380.9995 -lt 0.99is falseTogether they pin the value to a fraction strictly between 0 and 1 without hardcoding the
fixture's exact coverage. Both run against every installed Pester major (5.7.1 and 6.1.0).
0.01case on both Pestermajors) and pass after the fix
main— 425 passed, 12 failedThe same 12 failures occur on both, all from one container (
tests/Manifest.tests.ps1). Theyare the local psake 5.0.4
break-escape failure diagnosed in #155 and fixed in #162, which isstill unmerged. The +4 on this branch is the two new tests × two Pester majors.
PSScriptAnalyzer reports no new findings; the four warnings on the branch are all present on
main.Breaking Changes
None. The comparison is strictly more permissive than before: the old code only ever passed a
nonzero threshold at exactly 100% coverage, which the new code also passes. A build that
passed with a coverage threshold set still passes; builds that were failing incorrectly now
pass.
Notes for the reviewer
Outputpropertyrather than letting it fall through to the job's output stream, where the report lines were
interleaving with the result object. Existing assertions were reading through that array by
member enumeration and happened to work.
$totalis0if a counter ever reportsmissed="0" covered="0", which would throw a divide-by-zero mid-build. I have not seen Pester emit sucha counter and did not want to widen this fix on speculation. Happy to open a separate issue
if you think it is worth guarding.
CHANGELOG.mdappends to the same### Fixedblock as fix: Settle the severity threshold contract and retry analyzer crashes #163, so expect a trivial conflictdepending on merge order.
Closes#138