ci: handle missing comment artifact in GnuComment - #14657
Conversation
|
@cakebaker Could you take a look at it? Thanks! |
|
Right, I assume this is the error that this PR is fixing: TypeError: Cannot read properties of undefined (reading 'id')
at eval (eval at callAsyncFunction (/home/runner/work/_actions/actions/github-script/v9/dist/index.js:64949:16), <anonymous>:17:30)
at process.processTicksAndRejections (node:internal/process/task_queues:104:5)
at async main (/home/runner/work/_actions/actions/github-script/v9/dist/index.js:65100:20)
Error: Unhandled error: TypeError: Cannot read properties of undefined (reading 'id')Thanks for taking care of this! It would be really nice to no longer have that workflow showing up in red on the GitHub Actions dashboard. |
| return artifact.name == "comment" | ||
| })[0]; | ||
| if (!matchArtifact) { | ||
| core.info("No comment artifact found; skipping."); |
There was a problem hiding this comment.
Should we explain here that the it was likely because the workflow that was expected to produce it failed?
There was a problem hiding this comment.
Updated the log message to make it explicit that the artifact is likely missing because GnuTests failed or was cancelled.
There was a problem hiding this comment.
On second thoughts, would it make more sense to only run this workflow if the previous workflow succeeded, i.e.:
if: ${{ github.event.workflow_run.conclusion == 'success' }}
There was a problem hiding this comment.
I don't think we should restrict it to conclusion == 'success'. When a PR introduces GNU test failures or regressions, GnuTests intentionally exits with code 1 (failure), but it uploads the comment artifact via if: success() || failure() so GnuComment can still report the diff to the PR. If we require success, we wouldn't get comments on failing runs where the report is most needed.
The artifact is only missing when GnuTests fails early before reaching the comparison step (or is cancelled), which is why checking if (!matchArtifact) matches SizeComment and gracefully handles that case.
There was a problem hiding this comment.
You're absolutely right, although we can at least exclude cancelled workflows by adding:
if: |
github.event.workflow_run.conclusion == 'success' ||
github.event.workflow_run.conclusion == 'failure'There was a problem hiding this comment.
Makes sense! Added the condition to ensure we only trigger when the workflow conclusion is
either success or failure.
| return artifact.name == "comment" | ||
| })[0]; | ||
| if (!matchArtifact) { | ||
| core.info("No 'comment' artifact found (likely because GnuTests failed or was cancelled); skipping."); |
There was a problem hiding this comment.
you can update this comment now we know the conclusion could not have been "cancelled"
There was a problem hiding this comment.
Updated! Removed the mention of cancelled runs from the log message.
|
LGTM, thanks! |
|
GNU testsuite comparison: |
|
From 18 failed jobs to 1. |
This comment was marked as outdated.
This comment was marked as outdated.
|
cc @sylvestre |
f0a83b8 to
a93777a
Compare
a93777a to
d448100
Compare
|
@krosci Since this change could be tested per se, please keep an eye on the Actions dashboard to make sure everything runs as expected. |
3 participants
When GnuTests fails before producing the comment artifact, GnuComment crashes attempting to access properties of undefined. This adds a check for the artifact before downloading and unzips only if present.