From eff4ad9e63adf85f4b1da9f3d5a1126de1bba664 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Mon, 29 Jul 2024 16:50:51 +0100 Subject: [PATCH] chore: fix ci PRs might get merged with failing tests (#1689) We had a PR https://github.com/aws/jsii-compiler/pull/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 --- .github/workflows/build.yml | 8 ++++++-- projenrc/build-workflow.ts | 13 ++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ecd8ac2f..9425a61b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -138,9 +138,13 @@ jobs: permissions: {} env: CI: "true" + if: always() steps: - - name: Done - run: echo OK + - name: Build result + run: echo ${{needs.matrix-test.result}} + - name: Set status based on matrix build + if: ${{ needs.matrix-test.result != 'success' }} + run: exit 1 package: name: package needs: build diff --git a/projenrc/build-workflow.ts b/projenrc/build-workflow.ts index d0131a86..e12b56eb 100644 --- a/projenrc/build-workflow.ts +++ b/projenrc/build-workflow.ts @@ -230,7 +230,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' },