Surface empty BCPT results as a warning instead of silent success#2229
Surface empty BCPT results as a warning instead of silent success#2229jeffreybulanadi wants to merge 2 commits intomicrosoft:mainfrom
Conversation
When BCPT tests ran but produced no log entries (e.g. because the test runner exited early or the suite codeunit IDs did not match), the results file contained no entries. GetBcptSummaryMD returned an empty string, the AnalyzeTests step wrote nothing to the job summary, and the build succeeded with no indication that performance tests did not produce output. Changes: - TestResultAnalyzer.ps1: Differentiate between results file not existing (return empty string) and results file existing but containing no entries (return an informational message so the GitHub summary shows a visible indication that no measurements were recorded). - AnalyzeTests.ps1: Emit a GitHub Actions warning annotation when the BCPT results file is present but GetBcptSummaryMD returns empty, as a defence-in-depth measure. - AnalyzeTests.Test.ps1: Add tests for ReadBcptFile and GetBcptSummaryMD when the file does not exist and when it exists but contains no entries. Fixes microsoft#2158 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves visibility when BCPT performance test runs produce an output file but no recorded measurements, avoiding a silent “success” with an empty job summary.
Changes:
- Update
GetBcptSummaryMDto return a user-facing message when the BCPT results file exists but contains no entries. - Add a defense-in-depth warning emission in
AnalyzeTests.ps1for cases where a BCPT results file exists but no summary content is produced. - Extend Pester tests to cover missing BCPT results files and empty-entry BCPT results files.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Actions/AnalyzeTests/TestResultAnalyzer.ps1 | Distinguishes missing BCPT results from empty BCPT results and returns a visible summary message for the latter. |
| Actions/AnalyzeTests/AnalyzeTests.ps1 | Adds a warning annotation path intended to flag “ran but no results” situations. |
| Tests/AnalyzeTests.Test.ps1 | Adds test coverage for missing/empty BCPT results cases and validates GetBcptSummaryMD behavior. |
| if (-not $testResultsSummaryMD -and (Test-Path -Path $bcptTestResultsFile -PathType Leaf)) { | ||
| OutputWarning "BCPT tests were run but produced no results. The BCPT test suite may have failed to start or the test codeunits exited without recording any measurements." | ||
| } |
There was a problem hiding this comment.
The new warning annotation block is effectively unreachable: GetBcptSummaryMD only returns an empty/falsey value when the results file does not exist, but this condition requires the file to exist (Test-Path ... -PathType Leaf). If the intention is to surface an empty-results warning annotation, consider keying this check off the parsed BCPT contents (e.g., ReadBcptFile returning an empty hashtable) or emitting the warning from GetBcptSummaryMD when it detects an empty results file.
| . (Join-Path $scriptRoot '../AL-Go-Helper.ps1') | ||
| . (Join-Path $scriptRoot 'TestResultAnalyzer.ps1') | ||
| $bcpt = ReadBcptFile -bcptTestResultsFile 'non-existent-file.json' | ||
| $bcpt | Should -BeNullOrEmpty |
There was a problem hiding this comment.
This test description says it verifies ReadBcptFile returns $null when the file is missing, but Should -BeNullOrEmpty would also pass for an empty hashtable/collection. To make the behavior under test explicit (and guard the $null vs empty distinction used by GetBcptSummaryMD), assert specifically that the value is $null.
| $bcpt | Should -BeNullOrEmpty | |
| $bcpt | Should -Be $null |
| It 'Test GetBcptSummaryMD returns warning message when file exists but contains no entries' { | ||
| . (Join-Path $scriptRoot '../AL-Go-Helper.ps1') | ||
| . (Join-Path $scriptRoot 'TestResultAnalyzer.ps1') | ||
| $emptyResultsFile = Join-Path ([System.IO.Path]::GetTempPath()) "$([GUID]::NewGuid().ToString()).json" | ||
| try { | ||
| $null | ConvertTo-Json -Depth 99 | Set-Content -Path $emptyResultsFile -Encoding UTF8 | ||
| $md = GetBcptSummaryMD -bcptTestResultsFile $emptyResultsFile | ||
| $md | Should -Not -BeNullOrEmpty | ||
| $md | Should -Match 'No BCPT results were recorded' | ||
| } |
There was a problem hiding this comment.
The “file exists but contains no entries” test writes JSON null to disk. BCPT result files appear to be JSON arrays (your helper uses @(...) | ConvertTo-Json), so an empty-results file would more realistically be [] (empty array). Using an empty array here would better match the real-world format and protect against differences in how ConvertFrom-Json handles null vs [].
…ray in test - TestResultAnalyzer.ps1: Emit OutputWarning directly inside GetBcptSummaryMD when the results file exists but contains no entries, so the annotation is always surfaced at the right call site. - AnalyzeTests.ps1: Remove the now-unreachable defensive warning block identified by the bot; the warning is handled in GetBcptSummaryMD. - AnalyzeTests.Test.ps1: Use Should -Be null (not -BeNullOrEmpty) to assert the exact return value of ReadBcptFile for a missing file. Use an empty JSON array '[]' instead of 'null' to better represent a real empty BCPT results file. Add Mock for OutputWarning to assert the warning is emitted when no entries are found. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Problem
When BCPT performance tests run but produce no log entries (for example, because the test runner exits early, the suite codeunit IDs do not match the published app, or the job fails silently), the results file contains no entries. \GetBcptSummaryMD\ returned an empty string, the \AnalyzeTests\ step wrote nothing to the job summary, and the build succeeded with no indication that performance tests did not produce output.
Fixes #2158
Changes
Actions/AnalyzeTests/TestResultAnalyzer.ps1
Actions/AnalyzeTests/AnalyzeTests.ps1
Tests/AnalyzeTests.Test.ps1
Testing
You can also run the Pester test suite locally:
\\powershell
Invoke-Pester Tests/AnalyzeTests.Test.ps1
\
All 10 tests should pass.