Skip to content

test: Add Clear-PSBuildOutputFolder coverage - #175

Merged
tablackburn merged 5 commits into
psake:mainfrom
MisterTriangle:test/94-clear-output-folder
Aug 27, 2026
Merged

test: Add Clear-PSBuildOutputFolder coverage#175
tablackburn merged 5 commits into
psake:mainfrom
MisterTriangle:test/94-clear-output-folder

Conversation

@MisterTriangle

Copy link
Copy Markdown
Contributor

Summary

  • I added focused Pester coverage for Clear-PSBuildOutputFolder.
  • I verified that it removes an existing output folder and handles a missing folder without throwing.

Testing

  • pwsh -NoProfile -File ./build.ps1 -Task Test -Bootstrap

Closes #94

@tablackburn tablackburn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — and welcome. This is exactly the kind of contribution the roadmap asked for:
#94 sits outside the v1.0.0 milestone, which only means it does not gate the release, and
#120 explicitly says contributions to these
test-backfill issues are welcome during the cycle. Closes #94 is right and should stay.

The tests themselves are good. They import the built module the way the rest of the suite does,
use $TestDrive so nothing needs cleaning up, and the two behaviors you picked — removes an
existing folder, tolerates a missing one — are the right first two.

I have updated the branch (it had fallen two commits behind main) and approved the workflow run,
so CI should report shortly. Nothing needed from you for either.

One test worth adding before this merges

The function's most important behavior is the one thing not covered yet:

[parameter(Mandatory)]
[ValidateScript({
        if ($_.Length -le 3) {
            throw ($LocalizedData.PathLongerThan3Chars -f $_)
        }
        $true
    })]
[string]$Path

with the comment just above it:

# Maybe a bit paranoid but this task nuked \ on my laptop. Good thing I was not running as admin.

That guard exists because this function once deleted a drive root. It is the highest-stakes line
in the module, it has no coverage anywhere in tests/ today, and a regression in it would be
genuinely destructive rather than merely wrong. Something like:

It 'refuses a path short enough to be a drive root' {
    { Clear-PSBuildOutputFolder -Path 'C:\' } | Should -Throw
}

Asserting on the message (PathLongerThan3Chars from PowerShellBuild/en-US/Messages.psd1) rather
than just that it throws would be even better, since it distinguishes the guard firing from some
unrelated failure.

Smaller: the removal test cannot see a missing -Recurse

$outputPath = Join-Path -Path $TestDrive -ChildPath 'Output'
New-Item -Path $outputPath -ItemType Directory -Force > $null

That directory is empty, and Remove-Item deletes an empty directory with or without -Recurse.
Putting a nested file inside it first would make the test exercise the recursive path that the
function actually relies on.

Neither of these is a criticism of what is here — they are the natural next two assertions, and I
would rather ask than add them myself, since it is your contribution. Happy to take it as is if you
would prefer; say the word and I will pick them up in a follow-up instead.

@MisterTriangle

Copy link
Copy Markdown
Contributor Author

Thank you for the detailed review and the welcome. I added both suggestions:

  • The removal test now creates Nested/marker.txt, so it exercises recursive removal.
  • I added the drive-root guard test and asserted on the localized validation message. I mocked Test-Path so the test cannot reach the real filesystem if that validation ever regresses.

The focused test file passes 3/3, and the full ./build.ps1 -Task Test -Bootstrap run passes as well. Thank you for updating the branch and approving the workflow run.

@tablackburn
tablackburn merged commit 80b3454 into psake:main Aug 27, 2026
6 checks passed
@tablackburn

Copy link
Copy Markdown
Contributor

Merged — thank you. Both suggestions landed, and the guard test came back better than what I asked
for.

Mocking Test-Path in the module scope so the body cannot reach the filesystem if the
ValidateScript guard ever regresses is a good instinct, and not one I had suggested. A test for a
guard that exists because this function once deleted a drive root should not itself be capable of
deleting a drive root while proving the point.

I checked that the mock is load-bearing rather than decorative, using a path long enough to pass the
guard so that only the mock could prevent removal — the directory survived, so it genuinely takes
effect inside the module. I deliberately did not verify it the obvious way, by deleting the guard
and re-running, since the cost of being wrong about the mock is the drive root.

Also worth noting for anyone reading later: 'C:\' is exactly three characters, so the length check
fires identically on Linux and macOS. No platform guard needed, and all six CI legs agreed.

This is the second outside contribution to the v1.0.0 cycle, after #164. Both closed test-backfill
issues that sit outside the milestone — which, to say it plainly, means they do not gate the
release, not that they are unwanted. Contributions like this are exactly what
#120 invites, and they are welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests: Clear-PSBuildOutputFolder

2 participants