[devops] Show "(Publish failed)" instead of broken VSDrops link when html report publish fails.#25233
[devops] Show "(Publish failed)" instead of broken VSDrops link when html report publish fails.#25233rolfbjarne wants to merge 3 commits intomainfrom
Conversation
…html report publish fails. When the "Publish to Artifact Services Drop" step times out or fails, the VSDrops link in the GitHub comment would point to a non-existent page. Now detect the failure by setting an output variable when the step does not succeed, and show "(Publish failed)" in plain text instead of a broken link. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the DevOps pipeline and GitHub comment formatting to avoid posting a broken VSDrops HTML report link when publishing to VSDrops fails/timeouts.
Changes:
- Add pipeline steps intended to detect VSDrops publish failures and expose the result as an output variable.
- Update test-results comment generation to display “(Publish failed)” instead of a VSDrops link when the publish failed.
- Plumb the new output variable through the stage-dependencies parsing into
TestResult.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/devops/automation/templates/tests/run-tests.yml | Adds steps around the VSDrops publish to capture status and attempt to set an output variable on failure. |
| tools/devops/automation/templates/mac/build.yml | Same status-capture/output-variable approach for mac build/test jobs publishing to VSDrops. |
| tools/devops/automation/scripts/TestResults.psm1 | Adds a flag to suppress the VSDrops link in GitHub comments and parses the new output variable from job outputs. |
| - bash: | | ||
| if [ "$JOB_STATUS_BEFORE_VSDROPS" != "$AGENT_JOBSTATUS" ]; then | ||
| echo "VSDrops publish changed job status from '$JOB_STATUS_BEFORE_VSDROPS' to '$AGENT_JOBSTATUS'" | ||
| echo "##vso[task.setvariable variable=VSDROPS_PUBLISHED;isOutput=true]Failed" | ||
| fi |
There was a problem hiding this comment.
The failure detection here relies on comparing $AGENT_JOBSTATUS before/after the VSDrops task. This will miss publish failures when the job status is already Failed or SucceededWithIssues before the publish runs (for example when Run tests fails, $AGENT_JOBSTATUS is already Failed, and a subsequent VSDrops publish failure won’t change it). In that case the script won’t set VSDROPS_PUBLISHED=Failed, and the GitHub comment will still render a broken VSDrops link.
Consider setting VSDROPS_PUBLISHED based on the publishVSDrops step result instead (now that it has a name:), e.g. a follow-up step with condition: failed('publishVSDrops') (or equivalent) that unconditionally sets the output variable when that step fails, independent of the overall job status.
| - bash: | | ||
| if [ "$JOB_STATUS_BEFORE_VSDROPS" != "$AGENT_JOBSTATUS" ]; then | ||
| echo "VSDrops publish changed job status from '$JOB_STATUS_BEFORE_VSDROPS' to '$AGENT_JOBSTATUS'" | ||
| echo "##vso[task.setvariable variable=VSDROPS_PUBLISHED;isOutput=true]Failed" | ||
| fi |
There was a problem hiding this comment.
Same issue as in the tests template: comparing $AGENT_JOBSTATUS before/after the VSDrops publish can’t reliably detect whether the publish step failed if the job was already Failed/SucceededWithIssues earlier. That can still produce a broken VSDrops link in the GitHub comment.
Use the publishVSDrops step’s result (now that it’s named) to drive VSDROPS_PUBLISHED, e.g. set the output variable in a step guarded by condition: failed('publishVSDrops') so it’s independent of the current job status.
| @@ -597,6 +603,7 @@ class ParallelTestsResults { | |||
| $platformKey = $outputs.Keys | Where-Object { $_.StartsWith($jobName + ".") -and $_.EndsWith(".TESTS_PLATFORM") } | |||
| $attemptKey = $outputs.Keys | Where-Object { $_.StartsWith($jobName + ".") -and $_.EndsWith(".TESTS_ATTEMPT") } | |||
| $titleKey = $outputs.Keys | Where-Object { $_.StartsWith($jobName + ".") -and $_.EndsWith(".TESTS_TITLE") } | |||
| $vsdropsPublishedKey = $outputs.Keys | Where-Object { $_.StartsWith($jobName + ".") -and $_.EndsWith(".VSDROPS_PUBLISHED") } | |||
| } | |||
There was a problem hiding this comment.
$vsdropsPublishedKey is collected without the same disambiguation used for $statusKey (which does Sort-Object | Select-Object -Last 1). If the outputs contain multiple *.VSDROPS_PUBLISHED entries (for example due to retries/attempts), $vsdropsPublishedKey may be an array and you may end up reading an arbitrary/older value.
Recommend applying the same selection strategy as TESTS_JOBSTATUS (sort + pick the last key) so VSDropsPublished reflects the most recent attempt deterministically.
| [string] GetDownloadLinks($testResult) { | ||
| $dropsIndex = "$($this.VSDropsIndex)/$($testResult.TestStage)$($testResult.Title)-$($testResult.Attempt)/;/tests/vsdrops_index.html" | ||
| $artifactUrl = "$Env:SYSTEM_TEAMFOUNDATIONCOLLECTIONURI$Env:SYSTEM_TEAMPROJECT/_apis/build/builds/$Env:BUILD_BUILDID/artifacts?artifactName=HtmlReport-$($testResult.TestStage)$($testResult.Title)-$($testResult.Attempt)&api-version=6.0&`$format=zip" | ||
| $downloadInfo = "[Html Report (VSDrops)]($dropsIndex) [Download]($artifactUrl)" | ||
| if ($testResult.VSDropsPublishFailed) { | ||
| $downloadInfo = "(Publish failed) [Download]($artifactUrl)" | ||
| } else { | ||
| $dropsIndex = "$($this.VSDropsIndex)/$($testResult.TestStage)$($testResult.Title)-$($testResult.Attempt)/;/tests/vsdrops_index.html" | ||
| $downloadInfo = "[Html Report (VSDrops)]($dropsIndex) [Download]($artifactUrl)" | ||
| } |
There was a problem hiding this comment.
This change adds new user-visible behavior (rendering "(Publish failed)" instead of the VSDrops link) but there’s no accompanying coverage in the existing Pester tests for TestResults.psm1 (see tools/devops/automation/scripts/TestResults.Tests.ps1). Adding a test case where VSDropsPublished is set to Failed would help prevent regressions in the GitHub comment formatting.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…tion.
- Use condition: failed('publishVSDrops') instead of comparing
$AGENT_JOBSTATUS before/after, which reliably detects publish
failures even when the job was already Failed.
- Add Sort-Object | Select-Object -Last 1 to VSDROPS_PUBLISHED key
lookup to handle retries/attempts deterministically.
- Change text from '(Publish failed)' to
'(:warning: Html Report Publish failed :warning:)'.
- Add Pester test for VSDrops publish failed behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #19379da] Build passed (Build packages) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #19379da] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #19379da] Build passed (Build macOS tests) ✅Pipeline on Agent |
When the "Publish to Artifact Services Drop" step times out or fails, the VSDrops
link in the GitHub comment would point to a non-existent page. Now detect the
failure by setting an output variable when the step does not succeed, and show
"(Publish failed)" in plain text instead of a broken link.