fix: prevent non-E2E labels from triggering workflow runs (#23459)
* fix: prevent non-E2E labels from triggering workflow runs - Add conditional logic to all jobs to skip when non-ready-for-e2e labels are added - Only allow ready-for-e2e label additions to trigger workflows - Preserve all other trigger types (opened, synchronize, reopened) - Fixes issue where labels like 'Improvements' cause unnecessary CI runs Resolves the annoyance where adding labels like 'Improvements' to PRs triggers full workflow runs including all checks, when only the ready-for-e2e label should trigger E2E tests and associated builds. Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * refactor: consolidate workflow conditional logic into environment variable - Replace repeated conditional logic with single SKIP_WORKFLOW env variable - Simplify all job conditions from complex expression to env.SKIP_WORKFLOW != 'true' - Maintain exact same functional behavior while improving maintainability - Reduces code duplication across 20+ jobs in the workflow This addresses feedback to consolidate the repeated conditional logic: github.event.action != 'labeled' || github.event.label.name == 'ready-for-e2e' The refactored approach is cleaner and easier to maintain. Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: update required job logic to handle skipped jobs correctly - Remove 'skipped' from failure conditions in required job - Add SKIP_WORKFLOW condition to required job logic - Prevents false failures when E2E jobs legitimately skip - Maintains proper failure detection for actual job failures This fixes the CI failure where the required job was treating legitimately skipped E2E jobs as failures when no ready-for-e2e label is present. Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Keith Williams
co-authored by
keith@cal.com <keithwillcode@gmail.com>
Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
keith@cal.com <keithwillcode@gmail.com>
Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent
9ceb5ea3c5
commit
e45f9c5a13
+27
-21
@@ -16,10 +16,14 @@ concurrency:
|
||||
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
|
||||
cancel-in-progress: true
|
||||
|
||||
env:
|
||||
SKIP_WORKFLOW: ${{ github.event.action == 'labeled' && github.event.label.name != 'ready-for-e2e' }}
|
||||
|
||||
jobs:
|
||||
changes:
|
||||
name: Detect changes
|
||||
runs-on: buildjet-2vcpu-ubuntu-2204
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' }}
|
||||
permissions:
|
||||
pull-requests: read
|
||||
outputs:
|
||||
@@ -43,10 +47,11 @@ jobs:
|
||||
needs: [changes]
|
||||
runs-on: buildjet-2vcpu-ubuntu-2204
|
||||
name: Check for E2E label
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' }}
|
||||
permissions:
|
||||
pull-requests: read
|
||||
outputs:
|
||||
run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && (github.event.action != 'labeled' || github.event.label.name == 'ready-for-e2e') }}
|
||||
run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && env.SKIP_WORKFLOW != 'true' }}
|
||||
steps:
|
||||
- name: Check if PR exists with ready-for-e2e label for this SHA
|
||||
id: check-if-pr-has-label
|
||||
@@ -94,130 +99,131 @@ jobs:
|
||||
deps:
|
||||
name: Install dependencies
|
||||
needs: [changes, check-label]
|
||||
if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/yarn-install.yml
|
||||
|
||||
type-check:
|
||||
name: Type check
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/check-types.yml
|
||||
secrets: inherit
|
||||
|
||||
lint:
|
||||
name: Linters
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/lint.yml
|
||||
secrets: inherit
|
||||
|
||||
unit-test:
|
||||
name: Tests
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/unit-tests.yml
|
||||
secrets: inherit
|
||||
|
||||
build-api-v1:
|
||||
name: Production builds
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/api-v1-production-build.yml
|
||||
secrets: inherit
|
||||
|
||||
build-api-v2:
|
||||
name: Production builds
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/api-v2-production-build.yml
|
||||
secrets: inherit
|
||||
|
||||
build-atoms:
|
||||
name: Production builds
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/atoms-production-build.yml
|
||||
secrets: inherit
|
||||
|
||||
build-docs:
|
||||
name: Production builds
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/docs-build.yml
|
||||
secrets: inherit
|
||||
|
||||
build:
|
||||
name: Production builds
|
||||
needs: [changes, check-label, deps]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/production-build-without-database.yml
|
||||
secrets: inherit
|
||||
|
||||
integration-test:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-api-v1, build-api-v2]
|
||||
if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/integration-tests.yml
|
||||
secrets: inherit
|
||||
|
||||
e2e:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-api-v1, build-api-v2]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/e2e.yml
|
||||
secrets: inherit
|
||||
|
||||
e2e-api-v2:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-api-v1, build-api-v2]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/e2e-api-v2.yml
|
||||
secrets: inherit
|
||||
|
||||
e2e-app-store:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-api-v1, build-api-v2]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/e2e-app-store.yml
|
||||
secrets: inherit
|
||||
|
||||
e2e-embed:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-api-v1, build-api-v2]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/e2e-embed.yml
|
||||
secrets: inherit
|
||||
|
||||
e2e-embed-react:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-api-v1, build-api-v2]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/e2e-embed-react.yml
|
||||
secrets: inherit
|
||||
|
||||
e2e-atoms:
|
||||
name: Tests
|
||||
needs: [changes, check-label, build, build-atoms, build-api-v2]
|
||||
if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
uses: ./.github/workflows/e2e-atoms.yml
|
||||
secrets: inherit
|
||||
|
||||
analyze:
|
||||
name: Analyze Build
|
||||
needs: [build]
|
||||
if: ${{ env.SKIP_WORKFLOW != 'true' }}
|
||||
uses: ./.github/workflows/nextjs-bundle-analysis.yml
|
||||
secrets: inherit
|
||||
|
||||
merge-reports:
|
||||
name: Merge reports
|
||||
if: ${{ !cancelled() && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ !cancelled() && env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
needs: [changes, check-label, e2e, e2e-embed, e2e-embed-react, e2e-app-store, e2e-atoms]
|
||||
uses: ./.github/workflows/merge-reports.yml
|
||||
secrets: inherit
|
||||
|
||||
publish-report:
|
||||
name: Publish HTML report
|
||||
if: ${{ !cancelled() && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
if: ${{ !cancelled() && env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }}
|
||||
permissions:
|
||||
contents: write
|
||||
issues: write
|
||||
@@ -228,7 +234,7 @@ jobs:
|
||||
|
||||
cleanup-report:
|
||||
name: Cleanup HTML report
|
||||
if: ${{ !cancelled() && needs.check-label.outputs.run-e2e == 'true' && (github.event.pull_request.merged == true || github.event.pull_request.state == 'closed') }}
|
||||
if: ${{ !cancelled() && env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && (github.event.pull_request.merged == true || github.event.pull_request.state == 'closed') }}
|
||||
permissions:
|
||||
contents: write
|
||||
issues: write
|
||||
@@ -262,5 +268,5 @@ jobs:
|
||||
runs-on: buildjet-2vcpu-ubuntu-2204
|
||||
steps:
|
||||
- name: fail if conditional jobs failed
|
||||
if: needs.changes.outputs.has-files-requiring-all-checks == 'true' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'skipped') || contains(needs.*.result, 'cancelled'))
|
||||
if: env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled'))
|
||||
run: exit 1
|
||||
|
||||
Reference in New Issue
Block a user