ci: test the PR merged into its target branch, not a stale snapshot - #1029
Draft
Benjamin Elder (BenTheElder) wants to merge 1 commit into
Draft
ci: test the PR merged into its target branch, not a stale snapshot#1029Benjamin Elder (BenTheElder) wants to merge 1 commit into
Benjamin Elder (BenTheElder) wants to merge 1 commit into
Conversation
actions/checkout already checks out refs/pull/N/merge on pull_request events, so CI tests the PR merged into its target branch rather than the branch tip. But that merge commit is computed when the event fires and is never recomputed: nothing re-triggers the run when the target branch moves on, so a green check can reflect a merge with a target branch that is hours or days stale. Two individually-green PRs that touch the same code can both pass and still break the branch they land on. Merge the live tip of the target branch into the checkout before building. Any conflict fails the job -- CI has no business guessing at a resolution. The merge starts from the checked-out merge ref rather than the PR head, so the merge base is the target tip as of the event and only conflicts newly introduced by the target moving surface, not everything since the fork. The merge is committed rather than left staged because hack/third_party/kubernetes/verify-generated.sh refuses to run on a dirty tree and checks HEAD out into a scratch worktree. fetch-depth: 0 is needed for a merge base; the default depth-1 checkout has none. TARGET_BRANCH is passed via env rather than interpolated into the run script, since branch names can carry shell metacharacters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR: It is already more or less the case that we test the as-merged state, but with the target branch state frozen at the time the PR is opened/pushed. If you do a fresh CI run (either explicitly, or re-running with flake fixes), it continues to use the same payload and does not pickup the current target branch ("main") HEAD.
One alternative is merge queues ... but I'm a touch hesitant, not sure they're mature ... https://news.ycombinator.com/item?id=47881672
This is one area where I miss https://prow.k8s.io 😅