Skip CLI E2E on drafts with a pipeline guard - #4706
Draft
mokagio wants to merge 1 commit into
Draft
Conversation
Alternative to #4700, which skips inside `run-cli-e2e-tests.sh` so the required `CLI E2E Tests` check still reports on drafts. That costs two macOS VMs, a Windows VM and a Linux container per draft build, spun up to run a script that exits immediately. This guards the steps instead, the way `E2E Tests` and `Performance Metrics` — also required checks — already do, so nothing boots at all. The price is that the check does not report on a draft, and marking the PR ready does not by itself produce a build that reports it: `build_pull_request_ready_for_review` is `false` on the `studio` pipeline, and the Checkov policy `BK_PIPELINE_DONT_REBUILD_ON_UNDRAFTING` requires it to stay `false`. A draft PR marked ready therefore reports the three guarded checks only on the next push. That is already true of the other two, and every studio PR merged in the last fortnight pushed at least once after being marked ready. --- Generated with the help of Claude Code, https://code.claude.com Co-Authored-By: Claude Code Opus 5 <noreply@anthropic.com>
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.
Related issues
How AI was used in this PR
Claude Code wrote the change and ran the verification below. I reviewed the diff.
Proposed Changes
Two ways to stop
CLI E2E Testsrunning on draft PRs. Pick one; close the other.run-cli-e2e-tests.sh, so the required check still reports on drafts. The cost is that Buildkite still boots two macOS VMs, a Windows VM and a Linux container per draft build, to run a script that exits in seconds.if:, exactly as theE2E Testsgroup directly above it already does. Nothing boots. The cost is that the required check does not report on drafts.I'd previously assumed the
if:route was closed off because a required check has to report on every build. It isn't:E2E TestsandPerformance Metricsare both required contexts ontrunktoday, and both already skip drafts through anif:. This PR movesCLI E2E Testsinto the same bucket rather than inventing a third pattern.The catch, stated plainly
Marking a draft ready does not produce a build that reports the skipped checks.
build_pull_request_ready_for_reviewisfalseon thestudiopipeline, and the Apps Infra Checkov policyBK_PIPELINE_DONT_REBUILD_ON_UNDRAFTINGrequires it to stayfalse. So a PR marked ready reports the guarded checks on the next push, not on the state change.That is already the status quo for two required checks, and in practice authors push again before merging: all five studio PRs merged in the last fortnight had a commit after
ready_for_review. But it does mean this PR takes the count of "required checks that go quiet on drafts" from two to three. If that trade is unwelcome, #4700 is the one to take.Testing Instructions
Steps the AI took
The
if:guard genuinely suppresses reporting. On PR #4689 — marked ready with no subsequent push — the head SHA has statuses forCLI E2E Tests,Lint,Unit Tests,Data Liberationandfastlane Helper Tests, and noE2E TestsorPerformance Metricsat all. One Buildkite build (21382), created by the push, none created by the ready transition.Both are required checks.
GET /repos/Automattic/studio/branches/trunk/protectionlists exactly:Lint,Unit Tests,E2E Tests,Performance Metrics,CLI E2E Tests.The rebuild-on-ready setting.
GET /v2/organizations/automattic/pipelines/studioreportsbuild_pull_request_ready_for_review: false, andsrc/buildkite/policies/inbuildkite-cipins it there.This PR is a draft on purpose. The
CLI E2E Testscheck should be absent from its status list — that absence is the whole tradeoff, visible rather than argued.Pre-merge Checklist