rusackas commented on code in PR #40719:
URL: https://github.com/apache/superset/pull/40719#discussion_r3351728231
##########
.github/workflows/superset-e2e.yml:
##########
@@ -269,3 +287,34 @@ jobs:
${{ github.workspace }}/superset-frontend/playwright-results/
${{ github.workspace }}/superset-frontend/test-results/
name: playwright-artifact-${{ github.run_id }}-${{ github.job }}-${{
matrix.browser }}--${{ steps.set-safe-app-root.outputs.safe_app_root }}
+
+ # workflow_run runs don't attach their checks to the originating PR, so post
+ # an aggregate commit status back onto the PR head SHA. Make THIS the
required
+ # status check in branch protection (in place of the individual E2E jobs).
+ report-status:
+ needs: [cypress-matrix, playwright-tests]
+ if: always() && github.event_name == 'workflow_run'
+ runs-on: ubuntu-24.04
+ permissions:
+ statuses: write
+ steps:
+ - name: Report aggregate E2E status to PR commit
+ uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 #
v9.0.0
+ with:
+ script: |
+ // 'skipped' is acceptable: the change-detector legitimately skips
+ // jobs when no relevant files changed. Only real failures fail.
+ const results = [
+ '${{ needs.cypress-matrix.result }}',
+ '${{ needs.playwright-tests.result }}',
+ ];
+ const ok = results.every((r) => r === 'success' || r ===
'skipped');
+ await github.rest.repos.createCommitStatus({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ sha: context.payload.workflow_run.head_sha,
+ state: ok ? 'success' : 'failure',
+ context: 'E2E / required',
+ description: ok ? 'E2E passed (or skipped)' : 'E2E failed',
+ target_url:
`${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`,
+ });
Review Comment:
Right — that is the intended behavior, and it is safe because the merge gate
is the `pre-commit checks` workflow itself, not the E2E/Cypress/Playwright
workflows. Those expensive workflows are deliberately not in the
required-status-check set; they only fan out via `workflow_run` after
pre-commit succeeds. So if pre-commit fails, the required pre-commit check is
red and blocks the merge directly — the gated E2E jobs being skipped (and
therefore never reporting) does not create a false green, because they were
never the thing branch protection waits on. The only way to land is a green
pre-commit run, which is exactly what then unlocks the E2E fan-out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
