Skip to content

fix: Fail the build when a Pester run fails for any reason#133

Merged
tablackburn merged 4 commits into
mainfrom
bugfix/pester-result-gate
Jul 7, 2026
Merged

fix: Fail the build when a Pester run fails for any reason#133
tablackburn merged 4 commits into
mainfrom
bugfix/pester-result-gate

Conversation

@tablackburn

@tablackburn tablackburn commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Same bug lineage as #31, #52, and #57.

Test Plan

Verified against the built module with crash fixtures, under both Pester majors:

  • Pester 5.8.0 (what CI bootstraps): discovery-crash file → build now fails (previously FailedCount=0, Result=Failed, build passed green); BeforeAll crash → build fails; healthy suite → build passes
  • Pester 6.0.0: same three scenarios, same outcomes
  • Repo psakeFile.ps1 gate verified both directions: a discovery-crash container fails the build (exit 1); a healthy run under Pester 5.8.0 passes (301 tests, exit 0)
  • Full repo suite green: 316 tests, 0 failed on the full CI matrix
  • CI green on the final revision (ubuntu / windows pwsh / windows PowerShell 5.1 / macOS)

Breaking Changes

None in the API. Behavior change by design: builds that previously reported success while a setup block or test container had crashed will now correctly fail. Downstream consumers whose suites have latent setup failures will see red builds — that is the bug being fixed.

🤖 Generated with Claude Code

https://claude.ai/code/session_011semwa5HU1BUKKL4RoWjKa

Test-PSBuildPester gated only on FailedCount, so a BeforeAll/AfterAll
that threw or a test file that errored during discovery left the count
at zero and the build passed despite tests never running. Gate on the
run's aggregate Result property instead, which Pester derives from all
failure categories (failed tests, failed blocks, failed containers).

This applies the shipped-function half of the fix discussed in #128;
that PR covers the repository's own psakeFile.ps1.

Verified against crash fixtures: a discovery-crash file yields
FailedCount=0 / Result=Failed and now throws; a BeforeAll crash still
throws; a healthy suite still passes. Full suite: 301 passed, 0 failed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011semwa5HU1BUKKL4RoWjKa

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Test-PSBuildPester to correctly fail the build when a Pester run fails due to any failure category (e.g., failed blocks or discovery/container failures), rather than gating only on FailedCount (failed It tests).

Changes:

  • Switch the build gate from $testResult.FailedCount -gt 0 to $testResult.Result -eq 'Failed' to capture block/container/discovery failures.
  • Add an Unreleased changelog entry describing the behavior change.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
PowerShellBuild/Public/Test-PSBuildPester.ps1 Fail the build based on Pester’s aggregate run Result, covering failures beyond individual tests.
CHANGELOG.md Document the fix in the Unreleased section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread CHANGELOG.md Outdated
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Test Results

    4 files  +  1    376 suites  +94   18s ⏱️ +3s
  316 tests ±  0    314 ✅ + 13   2 💤  - 13  0 ❌ ±0 
1 264 runs  +316  1 217 ✅ +314  47 💤 + 2  0 ❌ ±0 

Results for commit 19faeac. ± Comparison against base commit f59efcc.

♻️ This comment has been updated with latest results.

Address review feedback on #133: the Unreleased entry linked #128, but
this change lands via #133. Point the entry at #133 and note the
companion relationship to #128 explicitly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011semwa5HU1BUKKL4RoWjKa
tablackburn pushed a commit that referenced this pull request Jul 7, 2026
…sts (#128)

In Pester 5 a run has three independent failure counts. Gating only on
FailedCount misses FailedBlocksCount (BeforeAll/AfterAll) and
FailedContainersCount (files that fail to discover/load), so a failing
BeforeAll or an unloadable test file passes the build green. This adds
those counts to the gate in this repository's own psakeFile.ps1.

The same gap in the shipped Test-PSBuildPester function is fixed
separately in #133.

Co-authored-by: nohwnd <me@jakubjares.com>
claude added 2 commits July 7, 2026 22:09
Replace the three-counter sum merged in #128 with the aggregate Result
property, matching the gate in Test-PSBuildPester. Same behavior, one
source of truth for what "failed" means. Suggested by nohwnd in the
#128 discussion.

Verified both directions locally: a discovery-crash container fails the
build (exit 1), and a healthy run under Pester 5.8.0 passes (301
passed, exit 0).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011semwa5HU1BUKKL4RoWjKa
@tablackburn tablackburn merged commit 88abe93 into main Jul 7, 2026
9 checks passed
@tablackburn tablackburn deleted the bugfix/pester-result-gate branch July 7, 2026 22:22
tablackburn pushed a commit that referenced this pull request Jul 8, 2026
Bump ModuleVersion to 0.8.2 and convert the Unreleased changelog
section to [0.8.2] 2026-07-08. Single user-facing change: the
Test-PSBuildPester failure gate fix from #133.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011semwa5HU1BUKKL4RoWjKa
@tablackburn tablackburn mentioned this pull request Jul 8, 2026
2 tasks
tablackburn added a commit that referenced this pull request Jul 8, 2026
Bump ModuleVersion to 0.8.2 and convert the Unreleased changelog
section to [0.8.2] 2026-07-08. Single user-facing change: the
Test-PSBuildPester failure gate fix from #133.
tablackburn pushed a commit that referenced this pull request Jul 10, 2026
Add tests/Test-PSBuildPester.tests.ps1 (#102).
Every invocation runs in a Start-Job subprocess with a pinned inner
Pester version, and the scenarios run against both installed Pester
majors (5.x and 6.x) to verify the shipped function keeps supporting
Pester 5 consumers. Crash fixtures are generated into $TestDrive at
runtime, never checked in. Scenarios: healthy suite, failing test
(regression #52), BeforeAll crash and discovery crash (#128/#133 gate),
NUnit result output, and code coverage output path/format (regression
Add a second, named Pester dependency (PesterLegacy, 5.9.0) to
requirements.psd1 so CI installs both majors side by side.

Fix two latent Test-PSBuildPester bugs the tests exposed, test-first
(all 14 tests failed before the fixes, pass after):

- The finally block called Remove-Module with an empty -Name when the
  optional ModuleName parameter was omitted, raising a parameter-
  binding error that -ErrorAction SilentlyContinue cannot suppress.
- An unconditional Import-Module Pester -MinimumVersion 5.0.0 loaded
  the newest installed Pester on top of an already-loaded one, which
  crashes with a Pester.dll version conflict when two versions are
  installed side by side. The function now respects a loaded Pester
  (with a clear error below 5.0.0) and imports the newest installed
  version only when none is loaded.

Closes #102

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011semwa5HU1BUKKL4RoWjKa
tablackburn added a commit that referenced this pull request Jul 10, 2026
)

Add tests/Test-PSBuildPester.tests.ps1 (#102). Every invocation runs
in a Start-Job subprocess with a pinned inner Pester version, and the
scenarios run against both installed Pester majors (5.x and 6.x) to
verify the shipped function keeps supporting Pester 5 consumers. Crash
fixtures are generated into $TestDrive at runtime, never checked in.
Scenarios: healthy suite, failing test (regression #52), BeforeAll
crash and discovery crash (#128/#133 gate), NUnit result output, and
code coverage output path/format (regression #62).

Pester 5.9.0 is installed side by side with 6.0.0 through the new
install-only requirements.pester-matrix.psd1; the bootstrap must not
import it, because importing two Pester majors into one session
crashes with a Pester.dll version conflict.

Fix two latent Test-PSBuildPester bugs the tests exposed, test-first:

- The finally block called Remove-Module with an empty -Name when the
  optional ModuleName parameter was omitted, raising a parameter-
  binding error that -ErrorAction SilentlyContinue cannot suppress.
- An unconditional Import-Module Pester -MinimumVersion 5.0.0 loaded
  the newest installed Pester on top of an already-loaded one. The
  function now respects a loaded Pester (with a clear error below
  5.0.0) and imports the newest installed version only when none is
  loaded.

Closes #102
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.

3 participants