Skip to content

Also fail on Pester block/container failures (not just failed tests) - #663

Open
Jakub Jareš (nohwnd) wants to merge 1 commit into
microsoft:mainfrom
nohwnd:fix/pester5-count-blocks-containers
Open

Also fail on Pester block/container failures (not just failed tests)#663
Jakub Jareš (nohwnd) wants to merge 1 commit into
microsoft:mainfrom
nohwnd:fix/pester5-count-blocks-containers

Conversation

@nohwnd

@nohwnd Jakub Jareš (nohwnd) commented Jul 7, 2026

Copy link
Copy Markdown
Member

⚠️ This PR was generated by an AI agent (GitHub Copilot CLI). Please review accordingly.

I've been seeing this commonly across Pester 5 build scripts and I'm fixing it where it looks like it could hide real failures.

In Pester 5 a run reports three independent failure counts:

  • FailedCount — failed tests (It)
  • FailedBlocksCount — failed BeforeAll / AfterAll
  • FailedContainersCount — files that error during discovery / fail to load

Gating on only FailedCount means a BeforeAll that throws, or a test file that fails to load, leaves FailedCount = 0 and the build passes green despite real failures. Pester itself derives the overall pass/fail from the sum of all three.

This includes all three counts in the gate, and surfaces each count (failed tests, blocks, containers) in the reported output instead of only the failed test count.

@nohwnd
Jakub Jareš (nohwnd) force-pushed the fix/pester5-count-blocks-containers branch from 3da9f75 to 1c3d676 Compare July 7, 2026 12:37

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for picking this one up and improving edge-ai! The fix is the right call.
Things are looking good and almost ready for merged:

  1. Please ensure you use the .github/PULL_REQUEST_TEMPLATE.md as the PR description content.
  2. Three inline comments to address consistency follow-ups, the gate and the markdown table were updated, but a few sibling outputs are still keyed on FailedCount alone.

Comment thread scripts/Invoke-Pester.ps1
Set-CIOutput -Name 'test-result' -Value $(if ($result.FailedCount -eq 0) { 'passed' } else { 'failed' })
Set-CIOutput -Name 'test-result' -Value $(if (($result.FailedCount + $result.FailedBlocksCount + $result.FailedContainersCount) -eq 0) { 'passed' } else { 'failed' })
Set-CIOutput -Name 'test-count' -Value $result.TotalCount.ToString()
Set-CIOutput -Name 'fail-count' -Value $result.FailedCount.ToString()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Consistency (non-blocking): fail-count still reports only FailedCount. With the new gate above, a run can now emit test-result = 'failed' while fail-count = 0, which is contradictory for any consumer of these outputs. Suggest summing all three counters (or reporting them separately):

Set-CIOutput -Name 'fail-count' -Value (($result.FailedCount + $result.FailedBlocksCount + $result.FailedContainersCount)).ToString()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Pls repost the suggestions as actual suggestions and not just code blocks so they can be applied.

@rezatnoMsirhC Chris Montazer (rezatnoMsirhC) Jul 9, 2026

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.

Something like this?

Suggested change
Set-CIOutput -Name 'fail-count' -Value $result.FailedCount.ToString()
Set-CIOutput -Name 'fail-count' -Value (($result.FailedCount + $result.FailedBlocksCount + $result.FailedContainersCount)).ToString()

Katrien De Graeve (@katriendg) Bill Berry (@WilliamBerryiii) Marcel Bindseil (@bindsi) Allen Greaves (@agreaves-ms) Have we considered suggestion blocks instead of code-blocks where applicable for the PR Review and/or Code Review agents?

Comment thread scripts/Invoke-Pester.ps1
Comment on lines +97 to +98
| Failed Blocks | $($result.FailedBlocksCount) |
| Failed Containers | $($result.FailedContainersCount) |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Consistency (non-blocking)relates to test-summary.json generation just above this hunk (lines 78-79).

The markdown table now surfaces FailedBlocksCount / FailedContainersCount, but the persisted test-summary.json still selects only FailedCount, so the uploaded artifact disagrees with this table and the gate. Consider adding the two new counters there as well:

$result | Select-Object TotalCount, PassedCount, FailedCount, FailedBlocksCount, FailedContainersCount, SkippedCount, Duration |
    ConvertTo-Json | Set-Content (Join-Path $OutputPath 'test-summary.json')

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Pls repost the suggestions as actual suggestions and not just code blocks so they can be applied.

Comment thread scripts/Invoke-Pester.ps1
Comment on lines +109 to 110
if ($CI -and ($result.FailedCount + $result.FailedBlocksCount + $result.FailedContainersCount) -gt 0) {
exit 1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reliability / developer UX (non-blocking)relates to Get-FailedTest and the annotation loop above (lines 52-71, 83-86).

Now that the build fails on block/container failures, there's a gap: Get-FailedTest only walks It results, so a failure that trips this new exit 1 (a throwing BeforeAll/AfterAll, or a container that fails discovery/load) produces no Write-CIAnnotation — the developer sees a red build with only the summary counts and no inline pointer to the cause. Consider annotating failed containers so the new failure modes are actionable, e.g.:

foreach ($container in $result.Containers) {
    if ($container.Result -eq 'Failed') {
        Write-CIAnnotation -Level 'Error' `
            -Message "Container failed to run: $($container.Item)$($container.ErrorRecord.Exception.Message)" `
            -File $container.Item
    }
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Pls repost the suggestions as actual suggestions and not just code blocks so they can be applied.

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