Skip to content

Commit

Permalink
chore: fix ci PRs might get merged with failing tests (#1210)
Browse files Browse the repository at this point in the history
We had a PR #1191 that got
merged despite a failure in Node22 tests.

This happened because on a failure of one of the matrix builds, the job
that is unifying them is skipped. This is documented behavior:

![image](https://github.com/user-attachments/assets/c4eb7ae6-1b37-4f5e-91c4-65a5e5661514)

The fix is to make sure the combination step always runs and manually
check the status of the matrix build.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

(cherry picked from commit b06c142)
  • Loading branch information
mrgrain committed Jul 29, 2024
1 parent 7ede562 commit 2513a9c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
8 changes: 6 additions & 2 deletions .github/workflows/build.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion projenrc/build-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,18 @@ export class BuildWorkflow {
needs: ['matrix-test'],
permissions: {},
runsOn: ['ubuntu-latest'],
steps: [{ name: 'Done', run: 'echo OK' }],
if: 'always()',
steps: [
{
name: 'Build result',
run: 'echo ${{needs.matrix-test.result}}',
},
{
if: "${{ needs.matrix-test.result != 'success' }}",
name: 'Set status based on matrix build',
run: 'exit 1',
},
],
},
'package': {
env: { CI: 'true' },
Expand Down

0 comments on commit 2513a9c

Please sign in to comment.