Improve CI sentinel job for better branch protection (backport #2743) (#2746)

* Improve CI sentinel job for better branch protection (#2743)

Previously, failed jobs in the CI matrix would cause the sentinel job
(all-tests-passed) to be skipped, which for purposes of Github Actions
branch protection would count as "success". This allowed PRs with
failing CI to be merged. This new approach which uses two sentinel jobs
should not suffer from this same issue.

(cherry picked from commit cc507a84cb1c319f83f8cbb935347d7265b73387)

# Conflicts:
#	.github/workflows/test.yml

* Resolve backport conflicts

Co-authored-by: Jack Koenig <koenig@sifive.com>

1 files changed, 30 insertions, 3 deletions

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index bd27edb9..9ace173e 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -82,15 +82,42 @@ jobs:
         run: sbt integrationTests/test
 
   # Sentinel job to simplify how we specify which checks need to pass in branch
-  # protection and in Mergify
+  # protection and in Mergify. This job checks that all jobs were successful.
   #
   # When adding new jobs, please add them to `needs` below
+  check-tests:
+    name: "check tests"
+    needs: [ci, integration]
+    runs-on: ubuntu-20.04
+    if: success() # only run if all tests have passed
+    outputs:
+      success: ${{ steps.setoutput.outputs.success }}
+    steps:
+      - id: setoutput
+        run: echo "::set-output name=success::true"
+
+  # Related to check-tests above, this job _always_ runs (even if tests fail
+  # and thus check-steps is skipped). This two sentinel job approach avoids an
+  # issue where failing tests causes a single sentinel job to be skipped which
+  # counts as passing for purposes of branch protection.
+  #
+  # See: https://brunoscheufler.com/blog/2022-04-09-the-required-github-status-check-that-wasnt
   all_tests_passed:
     name: "all tests passed"
-    needs: [ci, integration]
     runs-on: ubuntu-20.04
+    if: always() # Always run so that we never skip this check
+    needs: check-tests
+      # Pass only if check-tests set its output value
     steps:
-      - run: echo Success!
+      - run: |
+          PASSED="${{ needs.check-tests.outputs.success }}"
+          if [[ $PASSED == "true" ]]; then
+            echo "All tests passed!"
+            exit 0
+          else
+            echo "One or more tests FAILED!"
+            exit 1
+          fi
 
   # sbt ci-release publishes all cross versions so this job needs to be
   # separate from a Scala versions build matrix to avoid duplicate publishing