Skip to content

ci(code-coverage): move push:main trigger to merge_group#810

Merged
vikrantpuppala merged 1 commit into
mainfrom
vikrant/code-coverage-merge-group
May 27, 2026
Merged

ci(code-coverage): move push:main trigger to merge_group#810
vikrantpuppala merged 1 commit into
mainfrom
vikrant/code-coverage-merge-group

Conversation

@vikrantpuppala
Copy link
Copy Markdown
Contributor

Summary

Replaces the push: main trigger on code-coverage.yml with merge_group. pull_request and workflow_dispatch stay as-is.

Why

Today the suite runs twice per PR-to-main lifecycle:

  1. On pull_request (fast feedback for the author while iterating).
  2. On push: main after the merge has already happened.

Run #2 is observational — by the time it fires, the merge is done. A failure can't block anything; it can only tell you "main is broken, go fix it." That's not protection; that's an alert.

merge_group fires while the queue is processing the PR (against a transient branch = current main + the queued PR diff, freshly merged). Same SHA-equivalent state that push:main would have tested, but a failure here actually ejects the PR from the queue before it damages main.

The principle (from this morning's audit):

  • pull_request = fast iteration for the author.
  • merge_group = the gate that protects main.
  • push:main = observational only, useful for post-deploy smoke tests or release artifact triggers — not for protective checks.

This PR moves the workflow from the third category back into the second.

Why this PR doesn't also make it required

The suite takes ~17 min and would be added to every queue merge, including PRs that don't touch driver code. Queue throughput would drop materially. The right move is:

  1. Land this PR. Watch a few merges; confirm the merge_group run posts a clean check.
  2. If main breakages slip past it (e.g., someone admin-merges despite a red merge_group result), then promote E2E Tests and Code Coverage to a required status check on the ruleset.

The current setup is: queue runs the suite, you see the result, but it's not gating. Strictly better than today (where the result lands after the merge), without adding queue-time cost.

Subtlety: coverage override

The SKIP_COVERAGE_CHECK PR-body override only applies under pull_request events (where github.event.pull_request.body is populated). Under merge_group, the variable resolves to empty, the grep returns no match, and the threshold is enforced. That's intentional — overrides are an author-time escape hatch, not a queue-time bypass. Documented inline.

Test plan

  • CI passes on this PR (will run pull_request event end-to-end, including coverage check).
  • After merge: confirm no push:main run of E2E Tests and Code Coverage fires.
  • On a follow-up PR going through the queue, confirm the merge_group run of test-with-coverage fires against the transient queue branch.

This pull request and its description were written by Isaac.

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>
@vikrantpuppala vikrantpuppala added this pull request to the merge queue May 27, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 27, 2026
@vikrantpuppala vikrantpuppala added this pull request to the merge queue May 27, 2026
Merged via the queue into main with commit 794f512 May 27, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants