WordPress / WordPress/contributor-toolkit

The PR-checkout journey measures layout mid-reflow, so it fails intermittently on CI

Open Beginner friendly
#478 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
JavaScript
Stars
36
Forks
13
Avg merge
23h 19m
Merged PRs (30d)
72

Description

What happens

a PR checkout keeps ticket work and later PR edits on their own branches (tests/e2e/journeys/pr-checkout.spec.js:35) fails intermittently on Journeys (macos-latest), at this assertion:

const contextBox = await activeContext.boundingBox();
const ticketBox = await page.getByText( `Working on ticket #${ TICKET }`, { exact: true } ).boundingBox();
const linkedPullRequestsBox = await page.getByText( 'Linked pull requests', { exact: true } ).boundingBox();
expect( ticketBox.y ).toBeLessThan( contextBox.y );

It took trunk red at 1c03422 (the merge of #472), failing both the first attempt and the retry.

Why it is the test and not the app

The recorded values say it:

attempt 1   Expected: < -286.90625   Received: -137.90625
retry #1    Expected: < -290.90625   Received: -240.90625

Two things follow. The coordinates are negative, so the elements were above the viewport: the page was scrolled when the boxes were measured. And the numbers differ between the attempt and the retry on identical code — a settled layout in the wrong order would produce the same pair twice. The page was still moving.

So this is three point-in-time measurements taken while the panel is still reflowing after a PR checkout, on a runner slow enough to lose the race. It is not a wrong order in the app.

Measured rather than assumed: the same journey passed 20 of 20 consecutive local runs on macOS with --retries=0, on trunk with #472 in, and passed on the next CI run of #473 on the same base.

Probably woken by #472, not caused by it

E2E on trunk was green for the eleven runs before #472 and red on the first run containing it, and #472 ([Change] Put the apply panel above the site's other tickets) changes exactly the vertical positions this assertion reads. That is circumstantial and worth stating as such: the fragility was already there and used to win the race every time. #472 updated the order assertion in tests/unit/ticket-branch-list.test.cjs; nobody noticed a second place pinning an order, in a journey.

The fix

Stop reading three boxes at one instant. Assert the order through something that retries until the layout is still — expect.poll over the three boundingBox() reads, so a slow reflow costs a retry instead of a failure.

Worth a sweep for the same shape elsewhere: a boundingBox() read straight after an action, with no polling around it, is the pattern rather than this one line.

Note

Trunk's own head stays red on this check until another commit lands, even though nothing is broken.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the failing journey in tests/e2e/journeys/pr-checkout.spec.js:35 and inspect the boundingBox() reads around the order assertion. Use the existing expect.poll approach described in the issue so the three measurements are retried while the layout settles. Run this journey on macOS with retries disabled and confirm the check remains stable after the PR checkout.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.