Skip to content

Commit

Permalink
feat(ci): Collector jobs should fail on dependent job failure (#33784)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
armenzg authored Apr 21, 2022
1 parent adac2e4 commit 0213000
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 5 deletions.
12 changes: 10 additions & 2 deletions .github/workflows/acceptance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,23 @@ jobs:
# workflows that generate artifacts succeed
needs: [acceptance, frontend, chartcuterie]
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
timeout-minutes: 20

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
id: visual-snapshots-diff
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:
api-token: ${{ secrets.VISUAL_SNAPSHOT_SECRET }}
gcs-bucket: 'sentry-visual-snapshots'
Expand Down
10 changes: 8 additions & 2 deletions .github/workflows/backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ jobs:
needs: files-changed
name: relay test
runs-on: ubuntu-20.04
timeout-minutes: 10
timeout-minutes: 20
strategy:
matrix:
python-version: [3.8.12]
Expand Down Expand Up @@ -443,6 +443,12 @@ jobs:
typing,
]
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
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
8 changes: 7 additions & 1 deletion .github/workflows/frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ jobs:
frontend-required-check:
needs: [typescript-and-lint, webpack]
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
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

0 comments on commit 0213000

Please sign in to comment.