Skip to content

Commit 32955b2

Browse files
ci(code-coverage): move push:main trigger to merge_group (#810)
Running the coverage suite on push:main is observational — by the time it fires, the merge has already happened and a failure can't block anything. Moving the same trigger to merge_group runs the suite against the queue's transient branch (current main + the queued PR diff, freshly merged), which is the same state push:main would have tested, but where a failure can actually eject the PR from the queue before it damages main. pull_request stays as-is so the PR author still gets fast feedback while iterating; the queue run is the gate. Not making this a required status check in the same PR — the suite takes ~17 min and would add that to every queue merge, even for PRs that don't touch driver code. Promote to required later if main breakages slip past the merge_group run without it being mandatory. The coverage override (SKIP_COVERAGE_CHECK in PR body) intentionally doesn't apply to merge_group runs — the override is an author-time escape hatch, not a queue-time bypass. github.event.pull_request.body resolves to empty under merge_group, which naturally degrades to "no override" without code changes. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent aeb655c commit 32955b2

1 file changed

Lines changed: 19 additions & 8 deletions

File tree

.github/workflows/code-coverage.yml

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,27 @@ permissions:
55
id-token: write
66

77
on:
8-
push:
9-
branches:
10-
- main
118
pull_request:
9+
merge_group:
1210
workflow_dispatch:
1311

12+
# `pull_request` gives the PR author fast feedback as they iterate.
13+
# `merge_group` runs the same suite against the queue's transient
14+
# branch (current main + the queued PR diff, freshly merged) and is
15+
# the run that actually protects main — by the time `push:main` fires,
16+
# the merge has already happened and the coverage check has no power
17+
# to block. Hence we deliberately don't subscribe to `push:main`.
18+
#
1419
# Serialise E2E runs per ref so a force-push (or a fast follow-up commit)
1520
# on a PR cancels the previous run instead of racing it against shared
1621
# warehouse state (Delta tables, UC Volume files, etc.).
1722
#
18-
# Pushes to main are NOT cancelled — each merge commit needs its own clean
19-
# CI signal so a regression on commit N doesn't get hidden by commit N+1
20-
# arriving seconds later. (Concurrent main runs can still collide on shared
21-
# state, but that's the cost of preserving per-commit signal; the
22-
# uuid-suffix conventions in the e2e tests are what keep them isolated.)
23+
# Merge-queue runs are NOT cancelled — each queue entry needs its own
24+
# clean CI signal so a regression on entry N doesn't get hidden by
25+
# entry N+1 arriving seconds later. (Concurrent queue runs can still
26+
# collide on shared warehouse state, but that's the cost of preserving
27+
# per-entry signal; the uuid-suffix conventions in the e2e tests are
28+
# what keep them isolated.)
2329
concurrency:
2430
group: e2e-${{ github.workflow }}-${{ github.ref }}
2531
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
@@ -63,6 +69,11 @@ jobs:
6369
- name: Check for coverage override
6470
id: override
6571
env:
72+
# PR_BODY is empty on `merge_group` (no pull_request payload).
73+
# That's intentional — coverage overrides are an author-time
74+
# escape hatch, not a queue-time bypass, so the queue run
75+
# always enforces the threshold regardless of the PR's
76+
# SKIP_COVERAGE_CHECK marker.
6677
PR_BODY: ${{ github.event.pull_request.body }}
6778
run: |
6879
OVERRIDE_COMMENT=$(echo "$PR_BODY" | grep -E "SKIP_COVERAGE_CHECK\s*=" || echo "")

0 commit comments

Comments
 (0)