fix: Stop docs and test tasks unloading the caller's modules - #226
Open
tablackburn wants to merge 2 commits into
Open
fix: Stop docs and test tasks unloading the caller's modules#226tablackburn wants to merge 2 commits into
tablackburn wants to merge 2 commits into
Conversation
Build-PSBuildMarkdown and Test-PSBuildPester both ended with Remove-Module -Name $ModuleName, which removes every loaded module of that name -- including a copy the caller loaded and neither function ever imported. Both sit on default task chains, so this reached essentially every consumer. Test-PSBuildPester was the worse of the two: its import is conditional on -ImportModule, which defaults to $false, while its removal was not, so on the default Test chain it removed a module it had never touched. Both now record what was loaded before they import, remove only the instance they created, and restore what they displaced. Scoping the removal alone is not enough -- Import-Module -Force independently evicts a copy loaded from the same path -- and -Force has to stay, because without it the PSModuleInfo reports the previous build's exported commands and the generated markdown would document a stale surface. Build-PSBuildMarkdown also drops -Global from its import. PlatyPS resolves the module through the PSModuleInfo it is handed rather than by name, so there is no session-state lookup for -Global to serve; the generated markdown is byte-for-byte identical without it. Resolves #221 Resolves #222 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011GYJrhbrDzqufaeMqD9QjT
Both fixes change observable behavior on upgrade: a build that used to silently unload a consumer's module stops doing so. The migration guide gains an entry per task, including how to tell whether you were affected and how to put back documentation tasks that were dropped to avoid it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011GYJrhbrDzqufaeMqD9QjT
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
Fixes #221 and #222, which share a root cause: both
Build-PSBuildMarkdownandTest-PSBuildPestertreatedRemove-Module -Nameas an undo for an import. It is not —it removes every loaded module with that name, including a copy the caller loaded and
the function never imported. Both sit on default task chains (
Build→BuildHelp→GenerateMarkdown, andTest→Pester), so a build silently emptied the session itran in.
PowerShellOrg/PSDependdropped the documentation tasks entirely rather thanlive with it.
Build-PSBuildMarkdownnow records what was loaded, removes only theinstance it imported, and restores what it displaced. That covers the zero-export path
too, where it warns and
returns without generating anything —returninsidetrystill runs the
finally.Test-PSBuildPesteronly removes what it actually imported. Its import isconditional on
-ImportModule(default$false) while its removal was not, so on thedefault
Testchain it unloaded a module it had never touched.Build-PSBuildMarkdownalso drops-Globalfrom its import.Three constraints, each measured on this branch:
Import-Module -Forceindependentlyevicts a same-path copy before the
finallyis reached — measuredsame-path: instances=1; original instance object still present? False. Hencecapture-and-restore rather than only changing the removal.
-Forcemust stay. Without it, importing an already-loaded module returns thecached
PSModuleInfo: measuredWITHOUT -Force: Get-AlphaversusWITH -Force: Get-Alpha, Get-Betaafter a rebuild.$moduleInfogoes straight toNew-MarkdownCommandHelp -ModuleInfo, so dropping it would silently document a stalecommand surface.
-Globalon the docs import is unnecessary. PlatyPS 1.0.3 resolves the modulethrough the
PSModuleInfoobject, never by name. Generated markdown comparedbyte-for-byte before and after:
Get-Widget.md,Set-Widget.md, andPSBuildTestFixture.mdallidentical=True. Note-Globalis kept on the restoreimport — an import issued from inside the module without it reaches only
PowerShellBuild's own session state, and the caller would still see nothing.
Test Plan
./build.ps1 -Task Test— 597 passed, 0 failed, 3 skipped.Six new tests, driven by a new
Invoke-PSBuildModuleEvictionProbefixture helper thatpreloads a module in a background job, invokes the command, and reports what is loaded
and callable on both sides of the call:
Get-Widgetstill callableFunctionsToExport = @()) generates nothing and evicts nothing-ImportModulenot passed leaves the caller's module completely alone-ImportModulepassed restores the caller's copy and removes its own (thecaller's copy and the copy under test are at different paths, so the count
distinguishes evicted / left behind / correct)
Red-before-green verified by reverting only the two source files: the four eviction tests
fail with
Expected 1, but got 0; the two "nothing loaded" tests pass either way, whichis their job — they guard against over-restoring.
Breaking Changes
None. This is a behavior fix, but it changes what a build does to your session on
upgrade, so it has
CHANGELOG.mdanddocs/migration-v0.8-to-v1.0.mdentries.One residual cost, documented there: the restored module is a fresh import, not literally
the caller's original instance, so a
PSModuleInforeference held across the call goesstale. That is the price of keeping
-Force, and much smaller than the commanddisappearing.