fix: Stop docs and test tasks unloading the caller's modules

CHANGELOG.md

@@ -108,6 +108,27 @@
 
 ### Fixed
 
+- [**#221**](https://github.com/psake/PowerShellBuild/issues/221),
+  [**#222**](https://github.com/psake/PowerShellBuild/issues/222)
+  A build no longer unloads modules from the session it runs in.
+  `Build-PSBuildMarkdown` and `Test-PSBuildPester` both ended with
+  `Remove-Module -Name <ModuleName>`, which removes *every* loaded module of that
+  name — including a copy you loaded yourself and neither function ever imported.
+  Both are on default paths: `Build` depends on `BuildHelp` depends on
+  `GenerateMarkdown`, and `Test` depends on `Pester`. `Test-PSBuildPester` was the
+  worse of the two, because 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. The symptom was a command that worked a
+  moment ago no longer being recognized, with nothing in the build output to explain
+  it; `PowerShellOrg/PSDepend` gave up generated documentation entirely rather than
+  live with it. Both functions now record what was loaded before they import,
+  remove only the instance they created, and restore what they displaced —
+  including on the zero-export path, where `Build-PSBuildMarkdown` warns and
+  returns without generating anything. `Build-PSBuildMarkdown` also no longer
+  imports with `-Global`: PlatyPS resolves the module through the `PSModuleInfo`
+  object rather than by name, so the import no longer reaches into your session
+  state at all. The generated markdown is unchanged.
+
 - [**#206**](https://github.com/psake/PowerShellBuild/issues/206)
   `Build-PSBuildModule -Compile` no longer compiles your working directory when no
   compile directories are given. `CompileDirectories` defaulted to `@()`, and
@@ -300,7 +321,7 @@
   that passed before may now correctly fail.
 - [**#96**](https://github.com/psake/PowerShellBuild/issues/96)
   `Test-PSBuildScriptAnalysis` no longer fails with a path-resolution error
   when `SettingsPath` is not supplied. An unsupplied path was forwarded to
   PSScriptAnalyzer as `-Settings ''`, which resolved against the current
   directory and threw before any analysis ran, so the function's own
   documented example could not run as written.

PowerShellBuild/Public/Build-PSBuildMarkdown.ps1

@@ -19,7 +19,7 @@
     .PARAMETER Overwrite
         Overwrite existing markdown files and use comment based help as the source of truth.
     .PARAMETER ExcludeDontShow
         Exclude the parameters marked with `DontShow` in the parameter attribute from the help content.
     .PARAMETER UseFullTypeName
         Indicates that the target document will use a full type name instead of a short name for parameters.
     .EXAMPLE
@@ -51,7 +51,22 @@
         [bool]$UseFullTypeName
     )
 
-    $moduleInfo = Import-Module "$ModulePath/$ModuleName.psd1" -Global -Force -PassThru
+    # Record every copy of the module the caller already had loaded, so the finally block can
+    # put the session back the way it was found. Two separate things reach a caller's module
+    # otherwise: -Force evicts a copy loaded from this same path before the finally block is
+    # ever reached, and the removal that used to run there was by name, which takes every
+    # loaded copy regardless of who imported it or from where (psake/PowerShellBuild#221).
+    #
+    # -Force nevertheless stays. Without it, importing an already-loaded module is a no-op that
+    # returns the cached PSModuleInfo, so ExportedCommands would describe the previous build's
+    # command surface and the generated markdown would silently document a stale module.
+    #
+    # -Global is deliberately absent. Microsoft.PowerShell.PlatyPS resolves the module through
+    # the PSModuleInfo passed as -ModuleInfo below rather than by name, so it never performs a
+    # session-state lookup that -Global would serve. The generated markdown is byte-for-byte
+    # identical without it, and the import stops reaching into the caller's session state.
+    $previouslyLoadedModule = @(Get-Module -Name $ModuleName)
+    $moduleInfo = Import-Module "$ModulePath/$ModuleName.psd1" -Force -PassThru
 
     try {
         if ($moduleInfo.ExportedCommands.Count -eq 0) {
@@ -149,6 +164,16 @@
     } catch {
         Write-Error ($LocalizedData.FailedToGenerateMarkdownHelp -f $_)
     } finally {
-        Remove-Module $ModuleName
+        # Remove only the instance this function imported. The zero-export path above returns
+        # from inside the try block, which still runs this, so a module with no exported
+        # commands generates nothing and leaves the caller's session untouched all the same.
+        Remove-Module -ModuleInfo $moduleInfo -Force -ErrorAction SilentlyContinue
+
+        # Restored into the global session state, because that is where a caller's copy lives.
+        # An import issued from inside this module without -Global would only reach
+        # PowerShellBuild's own session state, and the caller would still see nothing.
+        foreach ($restoredModule in $previouslyLoadedModule) {
+            Import-Module -ModuleInfo $restoredModule -Global -ErrorAction SilentlyContinue
+        }
     }
 }

PowerShellBuild/Public/Test-PSBuildPester.ps1

@@ -78,14 +78,27 @@ function Test-PSBuildPester {
         throw ($LocalizedData.PesterVersionNotSupported -f $loadedPester.Version)
     }
 
+    # Nothing is imported unless -ImportModule is passed, so these stay empty on the default
+    # path and the finally block below has nothing to undo (psake/PowerShellBuild#222).
+    $previouslyLoadedModule = @()
+    $importedModule = $null
+
     try {
         if ($ImportModule) {
             if (-not (Test-Path $ModuleManifest)) {
                 Write-Error ($LocalizedData.UnableToFindModuleManifest -f $ModuleManifest)
             } else {
-                # Remove any previously imported project modules and import from the output dir
-                Get-Module $ModuleName | Remove-Module -Force -ErrorAction SilentlyContinue
-                Import-Module $ModuleManifest -Force
+                # Remove any previously imported project modules and import from the output
+                # dir, so the tests run against the module that was just built rather than a
+                # copy the session happened to be holding. What the caller had loaded is
+                # recorded first and restored in the finally block; ModuleName guards the
+                # lookup because Get-Module rejects an empty -Name with a parameter-binding
+                # error that -ErrorAction SilentlyContinue cannot suppress.
+                if ($ModuleName) {
+                    $previouslyLoadedModule = @(Get-Module -Name $ModuleName)
+                    $previouslyLoadedModule | Remove-Module -Force -ErrorAction SilentlyContinue
+                }
+                $importedModule = Import-Module -Name $ModuleManifest -Force -PassThru
             }
         }
 
@@ -153,10 +166,21 @@ function Test-PSBuildPester {
         }
     } finally {
         Pop-Location
-        # ModuleName is optional; Remove-Module with an empty -Name raises a parameter-binding
-        # error that -ErrorAction SilentlyContinue cannot suppress.
-        if ($ModuleName) {
-            Remove-Module -Name $ModuleName -ErrorAction SilentlyContinue
+
+        # Remove only the instance this function imported. The removal used to run by name and
+        # unconditionally, so on the default path -- where ImportModule is $false and nothing
+        # is imported at all -- it unloaded whatever the caller happened to have loaded under
+        # that name. Keying on the imported instance also retires the empty -Name guard that
+        # used to stand here, because there is no name-based removal left to protect.
+        if ($importedModule) {
+            Remove-Module -ModuleInfo $importedModule -Force -ErrorAction SilentlyContinue
+        }
+
+        # Restored into the global session state, because that is where a caller's copy lives.
+        # An import issued from inside this module without -Global would only reach
+        # PowerShellBuild's own session state, and the caller would still see nothing.
+        foreach ($restoredModule in $previouslyLoadedModule) {
+            Import-Module -ModuleInfo $restoredModule -Global -ErrorAction SilentlyContinue
         }
     }
 }

docs/migration-v0.8-to-v1.0.md

@@ -63,6 +63,12 @@
 - [`$PSBPreference.Sign.SkipCertificateValidation` now has an effect](#psbpreferencesignskipcertificatevalidation-now-has-an-effect)
   — the escape hatch did nothing on 0.8.x; a build that failed on an expired
   certificate may now succeed by signing with it.
+- [The `GenerateMarkdown` task no longer unloads your module](#the-generatemarkdown-task-no-longer-unloads-your-module)
+  — building help emptied the session it ran in; if you dropped the docs
+  tasks to avoid that, you can put them back.
+- [The `Pester` task no longer unloads a module it never imported](#the-pester-task-no-longer-unloads-a-module-it-never-imported)
+  — the same defect on the default `Test` chain, where nothing was imported
+  to justify the removal at all.
 
 
 ## AI-assisted migration
@@ -860,7 +866,7 @@
 `-FromModule`, and `$psake.build_success` are all explicitly retained — this
 repository still uses all three. The breaks are:
 
 - `default.ps1` is no longer auto-detected — rename it to `psakefile.ps1`, or
   pass `-BuildFile`. PowerShellBuild's own convention has always been
   `psakeFile.ps1`, so this is unlikely to affect you.
 - The standalone `psake.ps1` and `psake.cmd` runners are gone — use
@@ -1012,6 +1018,84 @@
 
 Tracked in [#193](https://github.com/psake/PowerShellBuild/issues/193).
 
+### The `GenerateMarkdown` task no longer unloads your module
+
+On 0.8.x, `Build-PSBuildMarkdown` ended with
+`Remove-Module -Name <ModuleName>` in its `finally` block. `Remove-Module
+-Name` removes **every** loaded module with that name — including a copy
+you loaded yourself, from a different path, that the function never
+imported. Generating documentation emptied the session it ran in.
+
+This was on by default. `Build` depends on `BuildHelp`, which depends on
+`GenerateMarkdown`, so every consumer who did not override
+`$PSBBuildDependency` ran it on every build.
+
+On 1.0.0 the function records the copies you had loaded, removes only the
+instance it imported itself, and re-imports what it displaced — including
+on the zero-export path, where it warns
+`No commands have been exported. Skipping markdown generation.` and
+returns without generating anything.
+
+**Detection.** You were affected if a command that worked before your build
+stopped being recognized afterwards, in a session where nothing obviously
+removed it:
+
+    The term 'Get-Widget' is not recognized as a name of a cmdlet, function,
+    script file, or executable program.
+
+**If you worked around this by dropping the documentation tasks**, you can
+put them back. `PowerShellOrg/PSDepend` did exactly that:
+
+**Before (0.8.x):**
+
+```powershell
+# Skips BuildHelp (GenerateMarkdown) - Build-PSBuildMarkdown has a Remove-Module scope bug
+$PSBBuildDependency = @('StageFiles')
+```
+
+**After (1.0.0):**
+
+```powershell
+# The default is fine again; delete the override entirely
+```
+
+Two smaller notes. The restored module is a fresh import rather than
+literally your original instance, so a `PSModuleInfo` reference you were
+holding across the call goes stale — a much smaller problem than the
+command disappearing, and the only way to keep `-Force` refreshing the
+documented command surface. And the import no longer passes `-Global`:
+PlatyPS resolves the module through the `PSModuleInfo` object it is handed,
+never by name, so the import no longer reaches into your session state at
+all. The generated markdown is byte-for-byte unchanged.
+
+Tracked in [#221](https://github.com/psake/PowerShellBuild/issues/221).
+
+### The `Pester` task no longer unloads a module it never imported
+
+The same defect in a worse form. On 0.8.x, `Test-PSBuildPester` imported
+the module under test only when `-ImportModule` was passed, but removed it
+by name **unconditionally**. `$PSBPreference.Test.ImportModule` defaults to
+`$false`, and both task files forward it verbatim, so for every consumer who
+had not turned it on, the `Pester` task imported nothing and then removed
+whatever you happened to have loaded under that name. `Test` depends on
+`Pester`, so `./build.ps1` and `./build.ps1 -Task Test` both did it.
+
+On 1.0.0 the function removes only the instance it imported, and restores
+anything it displaced on the way in. When `-ImportModule` is not passed it
+now touches your loaded modules not at all.
+
+**Detection** is the same as the entry above: a command that was available
+before the build is not available after it. This one reaches more builds,
+because a consumer who turned documentation generation off still runs tests.
+
+No build-file change is needed. If you set
+`$PSBPreference.Test.ImportModule = $true` purely to make the removal
+symmetrical, that is no longer a reason to keep it — but leaving it on is
+harmless, and the module under test is still imported from the output
+directory, still displacing a stale copy for the duration of the run.
+
+Tracked in [#222](https://github.com/psake/PowerShellBuild/issues/222).
+
 ## Adding an entry (for PR contributors)
 
 Every breaking-change PR that lands in v1.0.0 must add an entry here for

tests/Build-PSBuildHelp.tests.ps1

@@ -139,6 +139,105 @@
         }
     }
 
+    Context 'Build-PSBuildMarkdown and the modules the caller had loaded' {
+
+        # psake/PowerShellBuild#221. The function imported the module it was documenting and
+        # then, in its finally block, called Remove-Module by name -- which removes every
+        # loaded module with that name, including a copy the caller loaded and this function
+        # never imported. Generating documentation therefore emptied the session it ran in.
+        # PowerShellOrg/PSDepend gave up the docs tasks entirely rather than live with it.
+        #
+        # These assert on the session rather than on the generated files, because the files
+        # were always correct; the session was the casualty.
+
+        It 'leaves a module the caller loaded from the documented path loaded and callable' {
+            # The blunt case: the caller and the docs build point at the same module, so
+            # Import-Module -Force displaces the caller's instance on the way in. Restoring it
+            # is the half of the fix that scoping the removal alone does not cover.
+            $scenario = New-PSBuildDocsScenario -Path $TestDrive -Name 'evictionsamepath'
+            $manifestPath = Join-Path -Path $scenario.ModulePath -ChildPath (
+                '{0}.psd1' -f $scenario.ModuleName
+            )
+
+            $probeParameter = @{
+                ModulePath            = $script:builtModulePath
+                CommandName           = 'Build-PSBuildMarkdown'
+                Parameter             = New-PSBuildMarkdownParameter -Scenario $scenario
+                ProbeModuleName       = $scenario.ModuleName
+                ProbeModuleManifest   = $manifestPath
+                ProbeCommandName      = 'Get-Widget'
+                ProbeCommandParameter = @{ Name = 'Sprocket' }
+            }
+            $probe = Invoke-PSBuildModuleEvictionProbe @probeParameter
+
+            $probe.Threw | Should -BeFalse
+            $probe.LoadedBefore | Should -Be 1
+            $probe.ProbeResultBefore.Name | Should -Be 'Sprocket'
+
+            $probe.LoadedAfter | Should -Be 1
+            $probe.ProbeResultAfter.Name | Should -Be 'Sprocket'
+        }
+
+        It 'leaves the caller''s module alone when the documented module exports nothing' {
+            # The zero-export path warns and returns without generating a single file, and
+            # `return` inside `try` still runs the `finally`. So the one case that produces no
+            # documentation at all used to be just as destructive as the case that does.
+            # The caller's copy is loaded from a different path here, which is what the
+            # name-based removal reached and a scoped removal does not.
+            $scenario = New-PSBuildDocsScenario -Path $TestDrive -Name 'evictionnoexport'
+            $documentedManifestPath = Join-Path -Path $scenario.ModulePath -ChildPath (
+                '{0}.psd1' -f $scenario.ModuleName
+            )
+            (Get-Content -Path $documentedManifestPath -Raw) -replace
+                "(?s)FunctionsToExport = @\(.*?\)", 'FunctionsToExport = @()' |
+                Set-Content -Path $documentedManifestPath
+
+            $callerModulePath = Copy-PSBuildTestFixture -Destination (
+                Join-Path -Path $TestDrive -ChildPath 'evictionnoexportcaller'
+            )
+            $callerManifestPath = Join-Path -Path $callerModulePath -ChildPath (
+                '{0}.psd1' -f $scenario.ModuleName
+            )
+
+            $probeParameter = @{
+                ModulePath            = $script:builtModulePath
+                CommandName           = 'Build-PSBuildMarkdown'
+                Parameter             = New-PSBuildMarkdownParameter -Scenario $scenario
+                ProbeModuleName       = $scenario.ModuleName
+                ProbeModuleManifest   = $callerManifestPath
+                ProbeCommandName      = 'Get-Widget'
+                ProbeCommandParameter = @{ Name = 'Sprocket' }
+            }
+            $probe = Invoke-PSBuildModuleEvictionProbe @probeParameter
+
+            $probe.Threw | Should -BeFalse
+            $probe.Warning -join ' ' | Should -Match 'no commands'
+            $scenario.LocalePath | Should -Not -Exist
+
+            $probe.LoadedBefore | Should -Be 1
+            $probe.LoadedAfter | Should -Be 1
+            $probe.ProbeResultAfter.Name | Should -Be 'Sprocket'
+        }
+
+        It 'leaves nothing loaded when the caller had nothing loaded' {
+            # The other side of restoring: putting back only what was there, so a session that
+            # started clean does not end up holding the module the docs were generated from.
+            $scenario = New-PSBuildDocsScenario -Path $TestDrive -Name 'evictionclean'
+
+            $probeParameter = @{
+                ModulePath      = $script:builtModulePath
+                CommandName     = 'Build-PSBuildMarkdown'
+                Parameter       = New-PSBuildMarkdownParameter -Scenario $scenario
+                ProbeModuleName = $scenario.ModuleName
+            }
+            $probe = Invoke-PSBuildModuleEvictionProbe @probeParameter
+
+            $probe.Threw | Should -BeFalse
+            $probe.LoadedBefore | Should -Be 0
+            $probe.LoadedAfter | Should -Be 0
+        }
+    }
+
     Context 'Build-PSBuildMAMLHelp' {
 
         BeforeAll {