Test-PSBuildPester unloads a module it never imported

Test-PSBuildPester removes the module named by -ModuleName whether or not it ever imported it. The import is conditional on -ImportModule; the removal is not.

What happens

try {
    if ($ImportModule) {                                              # line 82
        if (-not (Test-Path $ModuleManifest)) {
            Write-Error ($LocalizedData.UnableToFindModuleManifest -f $ModuleManifest)
        } else {
            Get-Module $ModuleName | Remove-Module -Force -ErrorAction SilentlyContinue
            Import-Module $ModuleManifest -Force
        }
    }
    ...
} 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  # line 159
    }
}

The finally guards on $ModuleName being non-empty. It does not guard on $ImportModule. When -ImportModule is not passed, the function imports nothing and still removes whatever the caller had loaded under that name.

Measured

A caller loads a module, then calls Test-PSBuildPester with -ModuleName but without -ImportModule:

BEFORE: Probe loaded = 1; Get-Alpha -> alpha
AFTER : Probe loaded = 0; Get-Alpha -> gone

Why it matters

This fires on the default path, not an opt-in one. ImportModule defaults to $false in build.properties.ps1 (line 85), and both task files forward $PSBPreference.Test.ImportModule verbatim:

# psakeFile.ps1:162 and IB.tasks.ps1:101
ImportModule = $PSBPreference.Test.ImportModule

So for every consumer who has not set $PSBPreference.Test.ImportModule = $true — which is every consumer who has not gone looking for it — the Pester task takes the branch where the function imports nothing, and then unloads their module anyway. Test depends on Pester, so ./build.ps1 and ./build.ps1 -Task Test both do this.

It is worse than the same defect in Build-PSBuildMarkdown (#221) in two ways. There, the function at least imported something before removing it, so the removal has an internal justification and only over-reaches by scope. Here there is nothing to clean up at all — the removal is pure side effect on a session the function did not touch. And the Pester task runs on far more builds than GenerateMarkdown does, since a consumer who disabled documentation generation still runs tests.

The consequence is the same either way: after a build, a command that worked beforehand is no longer recognised, with nothing in the build output explaining why.

Root cause is shared

Both functions treat Remove-Module -Name as an undo for an import. It is not — it removes every loaded module with that name, regardless of who loaded it, from where, or whether anything was loaded by this function at all. Whatever approach is chosen for Build-PSBuildMarkdown should be applied here as well, and the two are probably one change.

Options

1. Move the removal inside the if ($ImportModule) guard, and remove by ModuleInfo. Capture what the import produced, remove that, and restore anything the caller had loaded first. This is the minimum correct behaviour: clean up exactly what was created, and nothing else. Cheap, and it makes the finally guard say what it means.
2. Drop the removal entirely. If -ImportModule was passed, leaving the module imported is a defensible end state — the consumer explicitly asked for it to be loaded, and a test task that leaves the tested module loaded is not surprising. Simplest change, and it makes the function non-destructive at the cost of a session side effect that is at least additive.
3. Fix both functions together with a shared helper. A small private function that imports a module, records what was displaced, and restores it, used by both Test-PSBuildPester and Build-PSBuildMarkdown. More work, and it keeps the two from drifting apart the way the psake and Invoke-Build task files have.

(1) is the smallest change that is correct here. (3) is worth considering if Build-PSBuildMarkdown is being fixed at the same time, since the two would otherwise grow two different answers to the same question.

One caution for whichever is chosen: the comment above line 159 documents a real regression — Remove-Module with an empty -Name raises a parameter-binding error that -ErrorAction SilentlyContinue cannot suppress — and tests/Test-PSBuildPester.tests.ps1 has a test for it (does not error when ModuleName is not provided). Whatever replaces this block needs to keep behaving correctly when ModuleName is absent.

Related: #221, Build-PSBuildMarkdown, which has the same root cause and the same options.

Finde Test-PSBuildPester und untersuche den Import-Zweig um Zeile 82 sowie die Bereinigung um Zeile 159; lies anschließend tests/Test-PSBuildPester.tests.ps1, insbesondere den Fall missing-ModuleName. Vergleiche das zugehörige Build-PSBuildMarkdown-Issue, bevor du entscheidest, ob die Bereinigung gemeinsam genutzt werden sollte. Erledigt ist es, wenn ein vom Aufrufer bereits geladenes Modul verfügbar bleibt, wenn ImportModule false ist, während das bestehende Verhalten ohne ModuleName weiterhin erfolgreich ist.