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.

Localisez Test-PSBuildPester et examinez la branche d’importation autour de la ligne 82 ainsi que le nettoyage autour de la ligne 159, puis lisez tests/Test-PSBuildPester.tests.ps1, en particulier le cas missing-ModuleName. Comparez l’issue associée à Build-PSBuildMarkdown avant de décider si le nettoyage doit être partagé. C’est terminé lorsqu’un module déjà chargé par l’appelant reste disponible lorsque ImportModule vaut false, tandis que le comportement existant sans ModuleName continue de passer.