Browse Source

feat(ci): Collector jobs should fail on dependent job failure (#33784)

In an attempt to reduce required checks (#33408), we merged jobs into less workflows and used collector jobs that would be marked as required (#33455).

Unfortunately, if one of the jobs we depend on fails, the collector job would be marked as Skipped which satisfies the Github required check. This allowed for the introduction of a regression on a PR earlier on the week (see https://github.com/getsentry/sentry/runs/6064646921?check_suite_focus=true).

This change makes each required check have a step which verifies the result of all dependent jobs. If any of them fails, the collector job fails as well.
Armen Zambrano G 2 years ago
parent
commit
0213000718

+ 10 - 2
.github/workflows/acceptance.yml

@@ -309,15 +309,23 @@ jobs:
     # workflows that generate artifacts succeed
     # workflows that generate artifacts succeed
     needs: [acceptance, frontend, chartcuterie]
     needs: [acceptance, frontend, chartcuterie]
     name: triggers visual snapshot
     name: triggers visual snapshot
-    # Do not execute on forks or on master
-    if: github.head_repository.full_name == 'getsentry/sentry' && github.ref != 'refs/heads/master'
+    # This is necessary since a failed/skipped dependent job would cause this job to be skipped
+    if: always()
     runs-on: ubuntu-20.04
     runs-on: ubuntu-20.04
     timeout-minutes: 20
     timeout-minutes: 20
 
 
     steps:
     steps:
+      # If any jobs we depend on fail, we will fail since this checks triggers Visual Snapshots which is a required check
+      - name: Check for failures
+        if: contains(needs.*.result, 'failure')
+        run: |
+          echo "One of the dependent jobs have failed. You may need to re-run it." && exit 1
+
       - name: Diff snapshots
       - name: Diff snapshots
         id: visual-snapshots-diff
         id: visual-snapshots-diff
         uses: getsentry/action-visual-snapshot@v2
         uses: getsentry/action-visual-snapshot@v2
+        # Do not execute on forks or on master. Forks are handled in visual-diff.yml
+        if: github.head_repository.full_name == 'getsentry/sentry' && github.ref != 'refs/heads/master'
         with:
         with:
           api-token: ${{ secrets.VISUAL_SNAPSHOT_SECRET }}
           api-token: ${{ secrets.VISUAL_SNAPSHOT_SECRET }}
           gcs-bucket: 'sentry-visual-snapshots'
           gcs-bucket: 'sentry-visual-snapshots'

+ 8 - 2
.github/workflows/backend.yml

@@ -265,7 +265,7 @@ jobs:
     needs: files-changed
     needs: files-changed
     name: relay test
     name: relay test
     runs-on: ubuntu-20.04
     runs-on: ubuntu-20.04
-    timeout-minutes: 10
+    timeout-minutes: 20
     strategy:
     strategy:
       matrix:
       matrix:
         python-version: [3.8.12]
         python-version: [3.8.12]
@@ -443,6 +443,12 @@ jobs:
         typing,
         typing,
       ]
       ]
     name: Backend
     name: Backend
+    # This is necessary since a failed/skipped dependent job would cause this job to be skipped
+    if: always()
     runs-on: ubuntu-20.04
     runs-on: ubuntu-20.04
     steps:
     steps:
-      - run: 'echo "All required checks have passed and we meet the requirements." '
+      # If any jobs we depend on fail, we will fail since this checks triggers Visual Snapshots which is a required check
+      - name: Check for failures
+        if: contains(needs.*.result, 'failure')
+        run: |
+          echo "One of the dependent jobs have failed. You may need to re-run it." && exit 1

+ 7 - 1
.github/workflows/frontend.yml

@@ -191,6 +191,12 @@ jobs:
   frontend-required-check:
   frontend-required-check:
     needs: [typescript-and-lint, webpack]
     needs: [typescript-and-lint, webpack]
     name: Frontend
     name: Frontend
+    # This is necessary since a failed/skipped dependent job would cause this job to be skipped
+    if: always()
     runs-on: ubuntu-20.04
     runs-on: ubuntu-20.04
     steps:
     steps:
-      - run: 'echo "All required checks have passed and we meet the requirements." '
+      # If any jobs we depend on fail, we will fail since this checks triggers Visual Snapshots which is a required check
+      - name: Check for failures
+        if: contains(needs.*.result, 'failure')
+        run: |
+          echo "One of the dependent jobs have failed. You may need to re-run it." && exit 1