fix(build): target staged output for code coverage#19
Conversation
Pester reports 0% coverage on every module scaffolded from this template
because the coverage globs point at the source tree
(`{{ModuleName}}/{Public,Private}/*.ps1`) but the test files
Import-Module from the staged build output (`Output/<Name>/<Version>/`).
Pester treats the two paths as different files, so every executed line
counts as "missed."
The natural fix — using `$Env:BHBuildOutput` directly — does not work:
PowerShellBuild only rewrites that variable to the staged version path
later, inside its own tasks. At psake `properties`-evaluation time,
`$Env:BHBuildOutput` is still BuildHelpers' default `<root>/BuildOutput`,
which doesn't exist yet and would cause Pester to bail with
"Could not resolve coverage path."
Compute the staged path explicitly from the manifest version (read via
`$Env:BHPSModuleManifest`, which BuildHelpers populates before psake
runs). This also drops the `{{ModuleName}}` placeholder from the
coverage section entirely, since the path is now derived from the
already-substituted BuildHelpers env vars.
Verified downstream in JsmOperations: Pester goes from 0% / 75% to
100% / 75% on the same test suite, with no test changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA build script is updated to redirect code coverage scanning from source files to staged build output, deriving the target path from the module version and project name. ChangesBuild Coverage Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 23 minutes and 28 seconds.Comment |
Pester reported 0% coverage despite all 111 tests passing. Coverage
globs pointed at the source tree (`JsmOperations/{Public,Private}/*.ps1`)
but tests Import-Module from the staged build output
(`Output/JsmOperations/<Version>/`). Pester treats the two paths as
different files, so every executed line counted as "missed."
Using `$Env:BHBuildOutput` directly does not work — PowerShellBuild
only rewrites that variable to the staged version path later, inside
its own tasks. At psake `properties`-evaluation time it's still
BuildHelpers' default `<root>/BuildOutput`, which doesn't exist yet
and causes Pester to bail with "Could not resolve coverage path."
Compute the staged path explicitly from the manifest version (read via
`$Env:BHPSModuleManifest`, which BuildHelpers populates before psake
runs).
Coverage now reports 100% / 100% / 100% / 100%
(instruction / line / method / class) on the same 111 tests.
Backported upstream to the template at
tablackburn/PowerShellModuleTemplate#19.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
build.psake.ps1 (1)
23-23: 💤 Low valueMinor:
Join-Path+ string-interpolation style differs from[IO.Path]::Combineused elsewhere in the block.Lines 14, 29, and 30 all use
[IO.Path]::Combinefor deterministic, OS-neutral path assembly, but line 23 switches toJoin-Path … "Output/$Env:BHProjectName/$_moduleVersion"with embedded forward-slash separators. On Windows,Join-Pathreturns a backslash-rooted path that then gets a forward-slash suffix, producing mixed separators (…\Output/Name/1.0.0). PowerShell and Pester tolerate this, so it is not a functional bug, but the inconsistency within the samepropertiesblock is worth tidying.🔧 Optional: align with `[IO.Path]::Combine` style
- $_stagedOutput = Join-Path $PSScriptRoot "Output/$Env:BHProjectName/$_moduleVersion" + $_stagedOutput = [IO.Path]::Combine($PSScriptRoot, 'Output', $Env:BHProjectName, $_moduleVersion)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.psake.ps1` at line 23, The assignment to $_stagedOutput uses Join-Path with a forward-slash interpolated string which creates inconsistent separators; change the construction to use [IO.Path]::Combine with $PSScriptRoot, "Output", $Env:BHProjectName, and $_moduleVersion so it matches the other uses of [IO.Path]::Combine in this properties block (keep the same variables: $_stagedOutput, $PSScriptRoot, $Env:BHProjectName, $_moduleVersion).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@build.psake.ps1`:
- Line 23: The assignment to $_stagedOutput uses Join-Path with a forward-slash
interpolated string which creates inconsistent separators; change the
construction to use [IO.Path]::Combine with $PSScriptRoot, "Output",
$Env:BHProjectName, and $_moduleVersion so it matches the other uses of
[IO.Path]::Combine in this properties block (keep the same variables:
$_stagedOutput, $PSScriptRoot, $Env:BHProjectName, $_moduleVersion).
There was a problem hiding this comment.
Pull request overview
Adjusts PowerShellBuild/Pester code coverage configuration so coverage is collected against the staged build output (the same files that tests import), preventing Pester from reporting 0% due to source-vs-output path mismatches.
Changes:
- Compute the staged output path from the module manifest version and BuildHelpers env vars.
- Point
PSBPreference.Test.CodeCoverage.FilesatOutput/<Name>/<Version>/{Public,Private}/*.ps1instead of the source tree placeholder paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR #19 review feedback: - Guard against missing $Env:BHPSModuleManifest / $Env:BHProjectName so bypassing build.ps1 (and Set-BuildEnvironment) fails fast with an actionable message instead of an obscure Import-PowerShellDataFile null-binding error. (Copilot review) - Switch the staged-output assembly from Join-Path with embedded forward slashes to [IO.Path]::Combine to match the rest of the properties block and avoid mixed separators on Windows. (CodeRabbit review) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai Good catch on the inconsistency — taken. Switched the staged-output assembly on Pushed in e445d7f. |
|
Tip For best results, initiate chat on the files or code changes.
∧_∧ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Pull in two follow-up polishes from tablackburn/PowerShellModuleTemplate#19 (the upstream template fix this branch already carries): - Guard against missing $Env:BHPSModuleManifest / $Env:BHProjectName so bypassing build.ps1 (and Set-BuildEnvironment) fails fast with an actionable message instead of an obscure Import-PowerShellDataFile null-binding error. - Switch the staged-output assembly from Join-Path with embedded forward slashes to [IO.Path]::Combine to match the rest of the properties block and avoid mixed separators on Windows. Verified locally: 100% Instruction/Line/Method/Class coverage, all tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(build): clarify PowerShellBuild bug-workaround status Three comment blocks in build.psake.ps1 referenced PowerShellBuild v0.7.3 bugs as "until bug is fixed" workarounds. Verified upstream status against PSB v0.8.0 (Feb 2026, latest) and updated the comments: 1. Coverage Threshold = 0 was framed as "until truncation bug is fixed." Reality: the truncation bug in Test-PSBuildPester.ps1:118 is display-only — the JaCoCo XML on disk has correct numbers and Codecov reads that. Threshold = 0 here is just to keep the local build from acting on the bogus 0% console display, the same "Codecov enforces threshold" pattern the upstream template uses. Reframed accordingly. 2. The custom ScriptAnalysis task (substitute for PSB's Test-PSBuildScriptAnalysis with its "$_Severity" typo) is still genuinely necessary — bug confirmed still in v0.8.0; no upstream fix PR open (only #106, which adds tests for the function). Updated version reference from 0.7.3 to v0.8.0 and noted the absence of a fix PR. No code change; just clarifying the rationale. Related to the multi-repo coverage-config alignment work in tablackburn/PowerShellModuleTemplate#19 and downstream PRs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(build): expand PSB and PR abbreviations in comments --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(build): align coverage config with template pattern Switch the staged-output path computation from a hardcoded module name to the env-var-driven pattern from tablackburn/PowerShellModuleTemplate#19: - Use $Env:BHPSModuleManifest / $Env:BHProjectName (populated by Set-BuildEnvironment before psake runs) instead of a hardcoded $moduleName = 'ReScenePS' and a manual manifest-path computation. - Add an env-var guard so direct Invoke-psake invocations (bypassing build.ps1) fail fast with an actionable message instead of an obscure Import-PowerShellDataFile null-binding error. - Use [IO.Path]::Combine for the staged path to avoid mixed separators on Windows, matching the rest of the properties block. Preserves the existing third coverage path (Classes/) — this module has a Classes/ subdirectory. Functionally equivalent to the prior config (same staged Output path, same coverage targets); this is a cosmetic alignment to the template pattern across the rest of the user's PowerShell modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: bootstrap BuildHelpers env vars before invoking psake Help.tests.ps1 and Manifest.tests.ps1 each have a "running outside ./build.ps1" escape hatch that calls Invoke-psake -TaskList Build directly. Until now the bootstrap only set BHBuildOutput later in the file; it relied on PowerShellBuild's Init task to populate the rest of the BHE env vars as a side effect. The new env-var guard in build.psake.ps1's properties block (added in the previous commit on this branch) runs *before* the Build task can do anything, so the isolated-test invocation path now fails fast with a guard error instead of bootstrapping. Add Set-BuildEnvironment to the bootstrap so BHPSModuleManifest and BHProjectName are populated before psake evaluates properties. Verified locally: with BH* env vars cleared, Invoke-Pester ./tests/Help.tests.ps1 now bootstraps cleanly (psake builds, 145 tests pass). Addresses Copilot review feedback on PR #11. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(build): drop $_ prefix from helper variable names Rename $_moduleVersion → $moduleVersion and $_stagedOutput → $stagedOutput per Copilot review feedback on PR #11. Pure rename, no behavior change. Same rename applied across PowerShellModuleTemplate, JsmOperations, and PlexAutomationToolkit so the user's modules stay in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(build): target staged output for code coverage
Pester reports 0% coverage because the coverage globs in build.psake.ps1
point at the source tree (PlexAutomationToolkit/{Public,Private}/*.ps1)
while the test files Import-Module from the staged build output
(Output/<Name>/<Version>/). Pester treats the two paths as different
files, so every executed line counts as "missed."
Compute the staged path explicitly from the manifest version (read via
$Env:BHPSModuleManifest, which BuildHelpers populates before psake
runs), and guard against missing env vars so direct Invoke-psake
invocations fail fast with an actionable message.
Same fix as upstream tablackburn/PowerShellModuleTemplate#19, the
template this module was scaffolded from.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(build): drop $_ prefix from helper variable names
Rename $_moduleVersion → $moduleVersion and $_stagedOutput → $stagedOutput
to match the upstream template change in
tablackburn/PowerShellModuleTemplate. Pure rename, no behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): bootstrap BuildHelpers env before Invoke-psake in test files
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pester reported 0% coverage despite all 111 tests passing. Coverage
globs pointed at the source tree (`JsmOperations/{Public,Private}/*.ps1`)
but tests Import-Module from the staged build output
(`Output/JsmOperations/<Version>/`). Pester treats the two paths as
different files, so every executed line counted as "missed."
Using `$Env:BHBuildOutput` directly does not work — PowerShellBuild
only rewrites that variable to the staged version path later, inside
its own tasks. At psake `properties`-evaluation time it's still
BuildHelpers' default `<root>/BuildOutput`, which doesn't exist yet
and causes Pester to bail with "Could not resolve coverage path."
Compute the staged path explicitly from the manifest version (read via
`$Env:BHPSModuleManifest`, which BuildHelpers populates before psake
runs).
Coverage now reports 100% / 100% / 100% / 100%
(instruction / line / method / class) on the same 111 tests.
Backported upstream to the template at
tablackburn/PowerShellModuleTemplate#19.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pull in two follow-up polishes from tablackburn/PowerShellModuleTemplate#19 (the upstream template fix this branch already carries): - Guard against missing $Env:BHPSModuleManifest / $Env:BHProjectName so bypassing build.ps1 (and Set-BuildEnvironment) fails fast with an actionable message instead of an obscure Import-PowerShellDataFile null-binding error. - Switch the staged-output assembly from Join-Path with embedded forward slashes to [IO.Path]::Combine to match the rest of the properties block and avoid mixed separators on Windows. Verified locally: 100% Instruction/Line/Method/Class coverage, all tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Pester reports 0% coverage on every module scaffolded from this template, because the coverage globs in
build.psake.ps1point at the source tree ({{ModuleName}}/{Public,Private}/*.ps1) while the test filesImport-Modulefrom the staged build output (Output/<Name>/<Version>/). Pester treats the two paths as different files, so every executed line counts as "missed."This fix points coverage at the staged output, derived from the manifest version and BuildHelpers env vars.
Why not just use
$Env:BHBuildOutput?That was my first attempt — it doesn't work. PowerShellBuild only rewrites
BHBuildOutputto the staged version path later, inside its own tasks. At psakeproperties-evaluation time, it's still BuildHelpers' default<root>/BuildOutput, which doesn't exist and causes Pester to bail withCould not resolve coverage path.So the path has to be computed explicitly from the manifest (
$Env:BHPSModuleManifestis populated bySet-BuildEnvironmentbefore psake runs).A side benefit: this drops the
{{ModuleName}}placeholder from the coverage section entirely — the path is now derived from the already-substituted BuildHelpers env vars.Verification
Verified downstream in
tablackburn/JsmOperations(which was scaffolded from this template). Same test suite, same source code — only this change applied:Test plan
Initialize-Template.ps1and confirm./build.ps1 -Task Testreports non-zero coverage{{ModuleName}}source folder still exists pre-init; the new code reads$Env:BHPSModuleManifestwhich BuildHelpers resolves from whichever*.psd1it auto-detects)🤖 Generated with Claude Code
Summary by CodeRabbit