test: Baseline the help building functions - #170
Conversation
Part of #105, PlatyPS step 2a. Nothing consumer-facing changes. Microsoft.PowerShell.PlatyPS 1.0.3 goes into an install-only requirements file rather than requirements.psd1, because build.ps1 bootstraps that file with -Import. The two PlatyPS modules cannot both be imported into one session: each loads its own YamlDotNet.dll through NestedModules with a different assembly identity, so whichever imports second fails with "Assembly with same name is already loaded". The failure is symmetric -- neither order works on PowerShell 7 -- and only a separate process escapes it. requirements.pester-matrix.psd1 already existed for the same shape of problem with the Pester majors, so it is renamed to the general requirements.install-only.psd1 rather than adding a second bespoke file. The three Build-PSBuild*Help functions had no tests, and the repository does not run its own docs tasks, so nothing observed them at all. The new baseline covers them against current platyPS 0.14.2 behavior, giving the migrations in 2b/2c/2d something to regress against. Each invocation runs in a subprocess, which the assembly conflict makes mandatory once the old and new implementations coexist. Writing the baseline surfaced #169: Build-PSBuildUpdatableHelp cannot succeed as wired. Its assertions are written here and skipped with a pointer, so they are not written twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012AKaM9i6NyMFDcJNeC34h5
There was a problem hiding this comment.
Pull request overview
This PR installs Microsoft.PowerShell.PlatyPS 1.0.3 as an install-only dependency and adds subprocess-isolated baseline tests for help generation.
Changes:
- Generalizes the install-only dependency manifest.
- Updates bootstrap installation.
- Adds Markdown, MAML, and updatable-help baseline tests.
- Adds a required regression for
GenerateUpdatableHelptask wiring, which currently omits-Module.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Summary |
|---|---|
tests/Build-PSBuildHelp.tests.ps1 |
Adds help-generation baseline tests. |
requirements.pester-matrix.psd1 |
Replaced by the generalized install-only manifest. |
requirements.install-only.psd1 |
Defines install-only Pester and PlatyPS dependencies. |
build.ps1 |
Installs the generalized manifest without importing modules. |
Suppressed comments (2)
tests/Build-PSBuildHelp.tests.ps1:233
- These skipped CAB success assertions do not provide the module-output path containing the MAML generated above (
$cabScenario.OutputPath). Issue #169 notes that fixing the undefined$moduleOutDirrequires an explicit parameter; once that parameter is added, this invocation will either fail binding or leave the actual CAB input path untested. Pass the module-output path in this scenario (while keepingOutputPathas the CAB destination).
$script:cabResult = Invoke-PSBuildCommandJob -CommandName 'Build-PSBuildUpdatableHelp' -Parameter @{
DocsPath = $script:cabScenario.DocsPath
OutputPath = $script:updatableHelpOutputPath
Module = $script:fixtureName
}
tests/Build-PSBuildHelp.tests.ps1:260
- On the supported Windows PowerShell 5.1 leg,
[IO.Path]::Combine($moduleOutDir, $locale)throws for the undefined$moduleOutDirbeforeNew-ExternalHelpCabcan bind its parameters. That exception reportspath1(typicallyValue cannot be null), notLandingPagePathorCabFilesFolder, so this baseline test fails on that CI leg instead of accepting the current defect. Includepath1in the accepted failure markers, or assert the exception type as well.
$script:cabResult.ErrorMessage | Should -Match 'LandingPagePath|CabFilesFolder'
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $script:cabResult = Invoke-PSBuildCommandJob -CommandName 'Build-PSBuildUpdatableHelp' -Parameter @{ | ||
| DocsPath = $script:cabScenario.DocsPath | ||
| OutputPath = $script:updatableHelpOutputPath | ||
| Module = $script:fixtureName | ||
| } |
There was a problem hiding this comment.
Correct, and a good catch — this is sharper than the issue it points at. Recorded on #169 as a fourth acceptance criterion: #169 (comment)
The three function-level defects could all be fixed and GenerateUpdatableHelp would still be broken, because the task passes neither -Module nor the module output directory that the undefined $moduleOutDir needs. This baseline passes -Module explicitly, so by construction it cannot catch that.
Not fixed here. #170 is a baseline of current behavior against the current platyPS, and the task wiring is #152's to repair along with the function — the tests for it belong with the fix that makes them pass, not with the baseline. Exercising the task also needs the child-process pattern from tests/build.tests.ps1, since psake cannot nest.
One correction on the other comment in this review, about line 260: it predicted the Windows PowerShell 5.1 leg would fail because [IO.Path]::Combine($moduleOutDir, $locale) throws on path1 before parameter binding. It does not. PowerShell converts the null to an empty string for the Combine(string, string) overload, so the path collapses to the bare locale name and CabFilesFolder validation is what fails. Confirmed on the actual 5.1 leg:
CI / Run Tests (Windows PowerShell 5.1) [+] fails parameter binding on the cab step 6ms
Also worth flagging for anyone reading this thread later: this review was submitted against f6a0f48 and is stale. The dependency changes it describes were removed nine minutes later, and the helpers have since moved into tests/fixtures/FixtureHelpers.psm1.
The PlatyPS 1.x install was inherited from #149's premise that the three help functions migrate one at a time. That premise does not hold: the two modules cannot be imported into one session, and the markdown schema changes underneath Build-PSBuildMAMLHelp, so the migrations are atomic. With an atomic migration there is no window where both modules are needed. The swap -- requirements.psd1, the RequiredModules entry, and the six Get-Module platyPS PreConditions -- belongs in the migration commit itself. Installing 1.x ahead of it bought nothing, and the install-only file and rename existed only to serve that install. What is left is the baseline test, which needs only the platyPS 0.14.2 already pinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012AKaM9i6NyMFDcJNeC34h5
The helpers were defined inside the test file's BeforeAll, following the existing pattern in Test-PSBuildPester.tests.ps1 and Help.tests.ps1. That pattern makes them unreachable from any other test file, and this file's job runner is already a near-duplicate of the one in Test-PSBuildPester.tests.ps1 -- evidence that the reuse is real rather than hypothetical. fixtures/FixtureHelpers.psm1 is where the repository already keeps shared test helpers, so the three move there and the test file keeps only tests. New-PSBuildDocsScenario takes the root directory as a parameter rather than reading $TestDrive, which is a Pester construct and is not visible inside a module scope. The analyzer suppressions follow the form already used in Private/Remove-ExcludedItem.ps1. Two are for the New- verb on functions that either build a hashtable or set up a test fixture, and one is for the runspace rule, which does not model a param() block fed by -ArgumentList and so reports every job parameter as undeclared. Full suite: 461 passed, 0 failed. FixtureHelpers.psm1 analyzes clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012AKaM9i6NyMFDcJNeC34h5
That file carried its own Invoke-TestPSBuildPesterJob, defined in BeforeAll, which duplicated the job mechanics now in fixtures/FixtureHelpers.psm1: start a job, import the built module, invoke one command, capture rather than throw, clean the job up. Invoke-PSBuildCommandInJob gains a RequiredModule parameter -- a name-to-version map imported inside the job before the PowerShellBuild module -- which is what the Pester matrix needed and the only real difference between the two implementations. It reports back what actually loaded in LoadedModuleVersion, so the "honors the Pester version that is already loaded" test asserts on the observed version rather than the requested one, as it did before. Invoke-TestPSBuildPesterInJob keeps the call sites short. It supplies the three parameters every scenario passes and forwards the rest, so the ten call sites change only by naming the module path. AdditionalParameters is renamed to the singular AdditionalParameter, matching the module's other parameter names. Test-PSBuildPester.tests.ps1: 18 passed, 0 failed, both inner Pester majors exercised. Full suite: 461 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012AKaM9i6NyMFDcJNeC34h5
Invoke-PSBuildCommandInJob invoked the command as "& $commandName @parameter -ErrorAction Stop", while Invoke-TestPSBuildPesterInJob also carries ErrorAction inside the splatted hashtable, as the runner it replaced did. Supplying a parameter both ways is fatal on Windows PowerShell 5.1: Cannot bind parameter because parameter 'ErrorAction' is specified more than once. PowerShell 7 accepts it, so every pwsh leg passed and only the 5.1 leg failed -- 16 tests in Test-PSBuildPester.tests.ps1. Verifying locally on pwsh alone was not enough; this is the class of break that leg exists to catch. ErrorAction is now defaulted into the splat only when the caller has not already set it, which fixes the collision and lets a caller ask for different behavior. Verified on Windows PowerShell 5.1: 19 passed, 0 failed, 5 skipped. Verified on PowerShell 7.6.5: 28 passed, 0 failed, 4 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012AKaM9i6NyMFDcJNeC34h5
Closes #149. Part of #105 (PlatyPS migration), Phase 2 of #120.
One new test file, nothing else. No dependency changes, no production code changes.
Why
Build-PSBuildMarkdown,Build-PSBuildMAMLHelp, andBuild-PSBuildUpdatableHelphave notests. The repository does not run its own docs tasks either — the root
psakeFile.ps1goesInit → Clean → Build → Analyze → Pester → Publish and never invokes
GenerateMarkdown,GenerateMAML, orGenerateUpdatableHelp. Three functions that the PlatyPS migration is aboutto rewrite have nothing observing them, and CI could not report a regression in any of them.
This is the red-before-green baseline, written against current
platyPS0.14.2 behavior.How
It tests the three functions directly rather than through psake. Their parameters are explicit,
so no
$PSBPreferenceplumbing is needed, and it sidesteps psake nesting entirely. The fixtureis copied into
$TestDrive, so nothing undertests/is mutated.Every invocation runs in a
Start-Jobsubprocess. That is not incidental —platyPS0.14.2 andMicrosoft.PowerShell.PlatyPS1.x each load their ownYamlDotNet.dllthroughNestedModuleswith different assembly identities, so whichever imports second fails with "Assembly with same
name is already loaded", in either order, on PowerShell 7. A separate runspace does not escape
it; only a separate process does. Once the migration lands, old and new behavior can only be
compared across processes, so the harness is built that way now.
Wait-Jobcarries a timeout, so a hang fails the test instead of stalling CI with no output —the failure mode #167 produced.
The three helpers live in
tests/fixtures/FixtureHelpers.psm1, not in the test file. An earlierrevision defined them in
BeforeAll, following the existing pattern inTest-PSBuildPester.tests.ps1andHelp.tests.ps1— but that pattern makes them unreachablefrom any other test file, and this file's job runner was already a near-duplicate of the one in
Test-PSBuildPester.tests.ps1. The reuse is real, not hypothetical, so they belong in the modulethe repository already keeps shared test helpers in:
Invoke-PSBuildCommandInJobNew-PSBuildDocsScenarioNew-PSBuildMarkdownParameterBuild-PSBuildMarkdown's four mandatory[bool]parameters at their defaultsNew-PSBuildDocsScenariotakes the root as a parameter rather than reading$TestDrive, whichis a Pester construct and is not visible inside a module scope.
Test-PSBuildPester.tests.ps1is converged onto the same runner. It carried its ownInvoke-TestPSBuildPesterJobwith the same mechanics; the only real difference was importing apinned Pester version inside the job, which is now
Invoke-PSBuildCommandInJob -RequiredModule,a name-to-version map. It reports what actually loaded in
LoadedModuleVersion, so the "honorsthe Pester version that is already loaded" test still asserts on the observed version rather
than the requested one.
Invoke-TestPSBuildPesterInJobsupplies the three parameters everyscenario passes, so the ten call sites change only by naming the module path.
That file is now 76 lines lighter and has no nested functions. It passes 18/0 with both inner
Pester majors exercised.
FixtureHelpers.psm1analyzes clean. Its three suppressions follow the form already used inPrivate/Remove-ExcludedItem.ps1.10 passing, 4 skipped. Full local suite: 461 passed, 0 failed.
This turned up a real bug: #169
Build-PSBuildUpdatableHelpcannot succeed as wired. Three defects, none shadowed by theothers:
New-ExternalHelpCab -LandingPagePathneeds the module landing page, butBuild-PSBuildMarkdowncallsNew-MarkdownHelpwithout-WithModulePage, so it is nevergenerated.
CabFilesFolder = [IO.Path]::Combine($moduleOutDir, $locale)—$moduleOutDiris definednowhere in the module or the psake file, so the path collapses to the bare locale name.
[string]$Module = $ModuleNamedefaults from a caller-scope variable the task never sets,and the task passes no
-Module.The map lists #152 as the riskiest link because updatable help is "untested today." It is worse
than untested: there is no working behavior to port. #169 is now a sub-issue of #152.
The assertions for the fixed behavior are written here and skipped with a pointer to #169, so
they are not written twice. One test pins the current failure, so fixing #169 forces this file
to be revisited rather than leaving a silent pass.
What this PR deliberately does not do
An earlier revision added
Microsoft.PowerShell.PlatyPS1.0.3 to an install-only requirementsfile, per #149's original wording — "add 1.x alongside
platyPSso the two can coexistwhile the three functions are migrated one at a time."
That premise does not survive. The two modules cannot coexist in a session, and #150 changes the
on-disk markdown schema that an unmigrated
Build-PSBuildMAMLHelpwould be handed, so the threemigrations are atomic. With an atomic migration there is no window in which both modules are
needed: the code references exactly one PlatyPS at a time, in
requirements.psd1, theRequiredModulesmanifest entry, and sixGet-Module platyPS -ListAvailablePreConditionsacross
psakeFile.ps1andIB.tasks.ps1. All of those flip in one commit.So the dependency swap belongs in the migration commit, not ahead of it, and #153 — "remove the
old
platyPSdependency" — is not a separate step; it is the swap. Route notes on #120.Not included
No changelog or migration-guide entry. Per the "Changelog and guide scope" decision on #120,
internal changes get neither, and a test-only addition is not triggered by a consumer upgrading
PowerShellBuild.