Skip to content

ci(playwright): raise the chromium shard budget to 21 minutes to unblock the merge queue - #30784

Merged
chirag-madlani merged 2 commits into
mainfrom
ci/raise-chromium-shard-budget
Aug 1, 2026
Merged

ci(playwright): raise the chromium shard budget to 21 minutes to unblock the merge queue#30784
chirag-madlani merged 2 commits into
mainfrom
ci/raise-chromium-shard-budget

Conversation

@ShaileshParmar11

@ShaileshParmar11 ShaileshParmar11 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Every merge_group run of Postgresql PR Playwright E2E Tests is currently failing in plan-playwright, at the Build duration-aware shard plans step, before a single test runs:

Lane chromium needs more than 24 shards to stay within the 19-minute plan budget;
the heaviest shard is predicted at 19.2m
Process completed with exit code 1

This is not specific to any one PR. Today's failures: #30705, #30768, #30458, #30725, #30754 — all merge_group. pull_request_target runs pass, because targeted selection plans a far smaller test list; the merge queue runs PW_EXECUTION_MODE: full.

The result is that no PR can leave the merge queue, including several already approved with all required checks green.

Root cause

assign_lane_within_budget() grows the shard count until every shard fits shard_budget_ms_for_lane(). For chromium that is COMMON_SHARD_BUDGET_MS = 19m, and the count is capped by COMMON_MAX_SHARDS = 24. The chromium suite has grown to the point where 24 shards still leaves the heaviest at 19.2m — about 1% over — so the planner raises SystemExit.

It is also self-perpetuating. The planner seeds predictions from prior full-mode timing artifacts:

gh run download "$run_id" --pattern 'playwright-timing-history-full-*'

That found nothing across the last 50 successful runs (no valid artifacts found to download, ×8) and fell back to the committed .github/playwright/timing-baseline.json. The upload step names artifacts playwright-timing-history-${mode}-…, and recent successful runs only produce -targeted-. Full-mode history comes only from merge-queue runs — which are all failing at planning. So no fresh timings are ever produced, and the stale baseline keeps predicting 19.2m.

Fix

Raise COMMON_SHARD_BUDGET_MS from 19m to 21m.

At 24 shards the heaviest is 19.2m, so the allocation loop is guaranteed to converge at or before the existing ceiling — no change to COMMON_MAX_SHARDS is needed.

Budget headroom is unchanged in kind, only in degree:

Bound Value 21m shard
timeout … 25m npx playwright test (line ~1092) 25m fits, ~4m spare
playwright-ci-postgresql timeout-minutes 35m fits (21m + ~5–8m setup/teardown)

Two things reviewers should weigh:

  1. The common lane now sits 1 minute above the dedicated lanes (TARGET_MS = 20m) rather than 1 minute below. The old "one minute of reserve" framing no longer holds, so the test name and README wording were updated to match rather than left stale.
  2. The strict 20-minute TARGET_MS ceiling is not affected. It rejects any single atomic unit over 20m; the allocation budget bounds a whole shard. A 21m shard built from units each under 20m does not trip it. The README conflated the two, and is corrected here.

The alternative — raising COMMON_MAX_SHARDS past 24 — keeps shards short but consumes more runners per run. Happy to switch if that is preferred; the one-line change is symmetric.

Longer term (not in this PR)

Worth following up separately, since the planner will hit this again as the suite grows:

  • Widen the history pattern to playwright-timing-history-* (or accept targeted/impacted) so the planner is not dependent on a mode the merge queue cannot currently produce.
  • Refresh the committed timing-baseline.json.
  • Make the planner degrade to max shards with a warning instead of exit 1. A capacity-planning heuristic being able to hard-block the merge queue is the deeper problem here; the 1% overshoot was merely the trigger.

Type of change:

  • Bug fix

Tests:

.github/scripts/tests/77 passed, including the three assertions in test_playwright_ci_planning.py that hardcoded the 19-minute figure and were updated:

  • test_common_lane_carries_its_own_shard_budget (renamed from test_common_lane_keeps_one_minute_of_allocation_reserve, whose name asserted an invariant that no longer holds)
  • test_common_shards_enforce_the_twenty_one_minute_budget (renamed from …nineteen_minute…), including the SystemExit message match

This PR's own merge_group run exercises the fix, since the queue branch contains it.

Note: Validate PR Metadata will be red — there is no linked issue for this. It is not a required status check, so it does not gate the queue; link an issue or apply skip-pr-checks if you would rather have it green.

🤖 Generated with Claude Code

Greptile Summary

The PR raises the common Chromium shard budget from 19 to 21 minutes so full-suite merge-queue planning can converge within the existing 24-shard cap.

  • Updates planner documentation and inline rationale for the new budget.
  • Adds full-mode regression coverage for successful convergence at the shard ceiling and failure when the ceiling cannot hold the lane.
  • Updates the plan-writing boundary assertions to enforce the 21-minute budget.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported allocator-convergence coverage gap is fixed by a full-mode test that succeeds at the 24-shard ceiling with a predicted duration above 19 minutes and within the new 21-minute budget.

Important Files Changed

Filename Overview
.github/scripts/build_playwright_shards.py Raises the common Chromium allocation budget to 21 minutes while preserving the existing shard cap and dedicated-lane budgets.
.github/scripts/tests/test_playwright_ci_planning.py Adds the requested full-mode allocator regression test, which reaches 24 shards and requires the newly available 19-to-21-minute window.
.github/playwright/README.md Documents the updated allocation formula and distinguishes the per-shard budget from the 20-minute atomic-unit ceiling.

Reviews (2): Last reviewed commit: "test(playwright): cover full-mode alloca..." | Re-trigger Greptile

The chromium lane outgrew a 19-minute shard. At the COMMON_MAX_SHARDS
ceiling of 24 the heaviest shard is predicted at 19.2m, so
assign_lane_within_budget() raises SystemExit and full-mode planning
aborts before a single test runs. Every merge_group run today failed
this way (PRs #30705, #30768, #30458, #30725, #30754), while
pull_request_target runs pass because targeted selection is far smaller.

Raise COMMON_SHARD_BUDGET_MS from 19m to 21m. At 24 shards the heaviest
is 19.2m, so the loop is guaranteed to converge at or before the
ceiling. 21m stays inside the 25m `timeout` wrapper around
`npx playwright test` and the 35m playwright-ci-postgresql job clock,
leaving ~4m of headroom.

Note the common lane now sits 1m above the dedicated lanes rather than
1m below. The strict 20-minute TARGET_MS ceiling is unaffected: it
bounds a single atomic unit, not a shard, so a 21m shard built from
units each under 20m does not trip it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 1, 2026 07:01
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 1, 2026
Comment thread .github/scripts/tests/test_playwright_ci_planning.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR unblocks merge-queue Playwright runs by increasing the common (chromium) shard planning budget so shard allocation converges within the existing 24-shard cap, preventing plan-playwright from aborting before tests execute.

Changes:

  • Increase the common chromium shard budget from 19 minutes to 21 minutes in the shard planner.
  • Update CI planning unit tests to reflect the new 21-minute budget and updated error-message expectation.
  • Refresh Playwright planning documentation to describe the 21-minute common-lane budget and clarify the distinction between shard budgets and the 20-minute atomic-unit ceiling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
.github/scripts/build_playwright_shards.py Raises COMMON_SHARD_BUDGET_MS to 21 minutes to stop full-mode shard planning from hard-failing at the 24-shard cap.
.github/scripts/tests/test_playwright_ci_planning.py Updates assertions, test names, and error-message matches from 19-minute to 21-minute budget.
.github/playwright/README.md Updates the planning formula and narrative to reflect the 21-minute common-lane budget and clarify budget vs ceiling semantics.

Comment thread .github/scripts/build_playwright_shards.py Outdated
chirag-madlani
chirag-madlani previously approved these changes Aug 1, 2026
…ceiling

Addresses review on #30784.

assign_lane_within_budget() was only exercised in "targeted" mode, so
neither the full-mode convergence path nor the SystemExit at
COMMON_MAX_SHARDS had coverage -- the exact code path that took the
merge queue down. Add both:

- test_full_mode_chromium_converges_at_the_shard_ceiling builds a lane
  that needs the window above 19m and asserts the allocator converges
  at or before the ceiling. Verified as a real guard: with the budget
  reverted to 19m it fails with "needs more than 24 shards ... heaviest
  shard is predicted at 20.4m".
- test_full_mode_chromium_reports_a_lane_the_ceiling_cannot_hold pins
  the SystemExit path, which had no coverage at all.

Also reword the budget comment: ~4m of headroom is relative to the 25m
playwright timeout wrapper specifically, not to the 35m job clock, which
is looser and additionally absorbs setup/teardown.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/scripts/tests/test_playwright_ci_planning.py:106

  • The comment says reverting the budget to 19m would fail on the final assertion, but with the current implementation assign_lane_within_budget() will raise SystemExit at the shard ceiling (before the assertions) when the budget is too low. This makes the comment misleading for future maintainers.
    # The allocator must converge at or before the ceiling, and the resulting
    # plan genuinely needs the window above 19m -- so quietly reverting the
    # budget to 19m fails on the final assertion rather than only in CI.

@chirag-madlani
chirag-madlani disabled auto-merge August 1, 2026 07:44
@chirag-madlani
chirag-madlani merged commit 17d35f1 into main Aug 1, 2026
74 of 76 checks passed
@chirag-madlani
chirag-madlani deleted the ci/raise-chromium-shard-budget branch August 1, 2026 07:45
@gitar-bot

gitar-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Raises the common Chromium Playwright shard-planning budget to 21 minutes and updates planning tests and documentation accordingly, unblocking the merge queue. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants