Fix wait / sleep which is causing many PRs to fail performance tests in CI for reasons unrelated to the PR by danluu · Pull Request #77726 · WordPress/gutenberg

danluu · 2026-04-28T00:15:14Z

This PR was originally created to stop every PR from having a failing CI run due to performance tests timing out at 60 minutes, which was blocking every PR for the #77716 fuzzing project. There was as race with #77725 and that issue solved the problem. However, the approach is different and this PR eliminates the 3s wait that #77725 has in successful cases. This seems to speed up the entire Performance CI run by approximately 2.5 minutes (no doubt there's a lot of noise and the true speedup might be 2 minutes or 3 minutes and not 2.5 minutes).

I originally closed this as a dup, but on seeing that this version has some advantages over the other one, I'm gong to re-open it.

BEGIN AI GENERATED TEXT

The performance job is not timing out because the Site Editor test suite or the
Site Editor itself became 20+ minutes slower. It is timing out because the
visitSiteEditor() Playwright helper can wait a full 60 seconds for a canvas
loader that has already appeared and disappeared.

The direct trigger was #77443. That PR did not change Site Editor loading code,
the performance spec, or the visitSiteEditor() helper. It changed runtime
timing enough to expose an existing race in the helper.

The latent helper bug came from the sequence of earlier helper changes, most
importantly #68667. #68667 made a missed loader a successful fallback path, but
only after spending the full 60 second timeout waiting for the loader to become
visible.

Once #77443 was on trunk, PR performance jobs compared two branches that both
contained the timing trigger. The Site Editor loading test calls
visitSiteEditor() 11 times per branch, so one artificial 60 second wait per
iteration adds about 22 minutes to a full PR performance run.

Relevant PRs and commits

E2E: Ensure SE canvas loader appears before waiting for it to disappear #61629 / d301cf7d5152: added a Site Editor canvas loader wait to avoid
resolving visitSiteEditor() before large canvas content finished loading.
E2E: Fix canvas waiter in visitSiteEditor #61816 / 7e24ae4f91ee: limited that wait to Site Editor routes that are
expected to load the canvas. The PR description says the previous behavior
added about 2 minutes to each affected performance test.
Try: Fix e2e tests 'visitSiteEditor' helper #68534 / ad4da873ce4: moved setPreferences() before the loader wait. This
made the helper start waiting for the loader later in the page lifecycle.
Fix flaky navigation-frontend-interactivity e2e tests #68667 / 4085555f8ac: added the important latent bug. The helper tries to
wait for .edit-site-canvas-loader, .edit-site-canvas-spinner to become
visible with a 60 second timeout, catches the timeout, and then waits for the
editor content region. If the loader was already gone, the helper succeeds
after wasting 60 seconds.
Remove unused catch block variables across the codebase #76969 / 309dfc2e885: changed catch ( error ) to catch {}. This was
mechanical and did not change behavior.
Experiments: Rebuild the wp-admin Experiments screen on the wp-build routes pattern #77443 / 0f99b8a8044: rebuilt the Experiments screen. This is the PR that
changed timing enough for performance CI to hit the existing helper race.

What the helper does

On trunk before the fix, visitSiteEditor() contains this sequence:

await this.visitAdminPage( 'site-editor.php', query.toString() );

if ( ! options.showWelcomeGuide ) {
	await this.editor.setPreferences( 'core/edit-site', {
		welcomeGuide: false,
		welcomeGuideStyles: false,
		welcomeGuidePage: false,
		welcomeGuideTemplate: false,
	} );
}

if ( ! query.size || postId || canvas === 'edit' ) {
	const canvasLoader = this.page.locator(
		'.edit-site-canvas-loader, .edit-site-canvas-spinner'
	);

	try {
		await canvasLoader.waitFor( { state: 'visible', timeout: 60_000 } );
		await canvasLoader.waitFor( {
			state: 'hidden',
			timeout: 60_000,
		} );
	} catch {
		await this.page
			.getByRole( 'region', { name: 'Editor content' } )
			.waitFor();
	}
}

The catch block comment says "If the canvas loader is already disappeared, skip
the waiting", but the implementation cannot know that until after
canvasLoader.waitFor( { state: 'visible', timeout: 60_000 } ) has already
timed out.

The Site Editor performance loading test hits this path 11 times per branch:

const samples = 10;
const throwaway = 1;
const iterations = samples + throwaway;

await admin.visitSiteEditor( {
	postId: draftId,
	postType: 'page',
	canvas: 'edit',
} );

The test then records results.firstBlock from
metrics.getLoadingDurations().timeSinceResponseEnd, which is
performance.now() - responseEnd. That means the helper's 60 second wait is
included in the firstBlock metric even if the first block was already present.

Evidence from CI

The #77443 performance run compared the PR merge commit
cf45676490ae3900632a512f6deaa9ab259e2d54 against trunk and still finished,
but it was close to the 60 minute workflow limit. Its Site Editor summary showed
a signature 60 second firstBlock value on the PR side while server-side work
was normal:

site-editor firstBlock:
  cf45676490ae3900632a512f6deaa9ab259e2d54: 65353.75 ms
  trunk:                                    4077.75 ms
  change:                                  1502.69%

site-editor wpTotal:
  cf45676490ae3900632a512f6deaa9ab259e2d54: 420.25 ms
  trunk:                                    418.41 ms

site-editor wpDbQueries:
  cf45676490ae3900632a512f6deaa9ab259e2d54: 52
  trunk:                                    52

That rules out a PHP/server/DB slowdown as the main cause of the 60 second
number.

After #77443 landed, later PRs compared PR branches against trunk where both
sides had the timing trigger. For example, the #77675 performance run was
canceled at the 60 minute job timeout while running the second Site Editor
branch:

23:52:49 site-editor PR branch starts running tests
00:08:06 site-editor PR branch finishes running tests
00:08:26 site-editor trunk branch starts running tests
00:18:17 job canceled at the 60 minute workflow timeout

That is consistent with roughly 11 minutes of artificial loader-wait delay per
branch, plus normal setup/build/test time.

Local reproduction and double-check

I reproduced the behavior with the actual performance test path, not a reduced
direct page.goto() probe. With instrumentation added around the old
visitSiteEditor() helper on #77443's PR merge commit, the page was already
loaded before the 60 second wait started:

after_visit_admin_page:
  wallMs=1804
  sinceResponseEnd=1530
  loaderPresent=false
  editorRegion=true
  iframePresent=true
  iframeBlocks=1437

after_set_preferences:
  wallMs=1884
  sinceResponseEnd=1662
  loaderPresent=false
  editorRegion=true
  iframeBlocks=1437

before_loader_visible_wait:
  wallMs=1894
  sinceResponseEnd=1685
  loaderPresent=false
  editorRegion=true
  iframeBlocks=1437

loader_wait_catch:
  wallMs=61934
  sinceResponseEnd=61723
  waitError="TimeoutError: locator.waitFor: Timeout 60000ms exceeded."
  iframeBlocks=1437

reported firstBlock=61807ms

This answers the important question directly: yes, the content was present
before 60 seconds. The iframe already contained 1437 blocks before the helper
started the 60 second loader-visible wait.

I then ran the same instrumented path on the pre-#77443 base commit
90e45267257fc4a4da0095933ecc24fa3bae776d. There the loader was still present
when the helper started waiting, so the 60 second timeout was not hit:

after_visit_admin_page:
  wallMs=739
  sinceResponseEnd=513
  loaderPresent=true
  editorRegion=false
  iframePresent=false

before_loader_visible_wait:
  wallMs=755
  sinceResponseEnd=531
  loaderPresent=true

after_loader_visible_wait:
  wallMs=841

after_loader_hidden_wait:
  wallMs=1020

reported firstBlock=1306.7ms

This isolates the behavior change to #77443's merge comparison: the helper code
was the same, but the loader was missed after #77443 and not missed before it.

Why #77443 is the trigger but not the actual slowdown

The #77443 diff changes the Experiments screen and related settings plumbing:

lib/experimental/experiments/load.php
lib/experiments-page.php
lib/init.php
lib/load.php
package-lock.json
package.json
packages/e2e-test-utils-playwright/src/request-utils/gutenberg-experiments.ts
routes/experiments-home/api.ts
routes/experiments-home/package.json
routes/experiments-home/route.ts
routes/experiments-home/stage.tsx
routes/experiments-home/style.scss

It does not change these Site Editor or performance-test files:

packages/edit-site
packages/e2e-test-utils-playwright/src/admin/visit-site-editor.ts
test/performance/specs/site-editor.spec.js
packages/scripts/config/playwright.config.js
test/performance/playwright.config.ts

So the correct interpretation is:

Fix flaky navigation-frontend-interactivity e2e tests #68667 left a race in visitSiteEditor(): a missed loader costs 60 seconds
and then still succeeds.
Experiments: Rebuild the wp-admin Experiments screen on the wp-build routes pattern #77443 changed page/runtime timing enough that, in performance CI, the
loader was already gone by the time visitSiteEditor() started waiting for
it to become visible.
The performance harness records the helper delay as firstBlock.
Once Experiments: Rebuild the wp-admin Experiments screen on the wp-build routes pattern #77443 was on trunk, both compared branches hit the same delay, pushing
the job past the workflow's 60 minute timeout.

Fix direction

The helper should not require the loader to appear. It should wait for either:

the loader to be present, or
a ready editor/canvas state that proves the loader has already come and gone.

Then it should verify the loader is absent and the editor content/canvas is
ready. That preserves the intent of #61629 and #61816, while avoiding the
60-second penalty introduced by the fallback behavior from #68667.

END AI GENERATED TEXT

Note that this PR was AI generated (GPT-5.5)

github-actions · 2026-04-28T00:16:14Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @danluu! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

github-actions · 2026-04-28T01:43:19Z

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

Required label: Any label starting with [Type].
Labels found: First-time Contributor, [Package] E2E Tests.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

github-actions · 2026-04-28T01:43:21Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: danluu <danluu@git.wordpress.org>
Co-authored-by: dmsnell <dmsnell@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

dmsnell

these two tests both define isVisibleElement and isReadyElement and they look identical. let’s extract them? I think that will make the diff look much smaller, especially if we define these helpers somewhere in the module global scope so that they aren’t right there in the waitForFunction lines.

dmsnell · 2026-05-04T18:15:12Z

This seems a bit surprising to me, but the initial results look like this clearly demonstrates slowing down the performance tests runs.

The revert I made in #77895 seems to be running within around 39 min while this branch is closer to 42 min. I’m not sure why this seems to contradict the 2.5 min speedup you found; perhaps I made a mistake in creating the revert PR.

Before	After

danluu · 2026-05-04T18:17:46Z

Interesting. It's definitely possible this fix doesn't help. I ran it twice and got a speedup both times, but the process is fairly noise. If we wanted a better measurement, we could try running a bunch of times locally (which will be less accurate, but should have less noise and can directionally indicate what should happen), or add instrumentation that shows what's going on.

dmsnell · 2026-05-04T18:22:34Z

we could try running a bunch of times locally

I would be more curious about instrumentation, because I could believe there are platform differences in GitHub actions that lead to a different runtime locally vs. in the CI runners.

these jobs are more or less running simultaneously and both share the same merge-base with trunk — 264ba7b

perhaps we could log how long we wait for the different canvas events?

anyway I think I take that back; I would be very interested to know if running locally can reliably produce the opposite result.

by any chance, you weren’t comparing this branch against trunk were you? vs. this branch against 264ba7b?

danluu · 2026-05-04T18:27:07Z

At the time, I was comparing to trunk the moment #77725 was merged and with this applied to trunk before #77725. Another thought is that, when doing a comparison for this, we could run only the part that's impacted by this, which will reduce the noise from unrelated parts of the job (which is where most the run time is from and, I would guess, where most of the noise is coming from).

danluu · 2026-05-04T18:52:26Z

I tried running the relevant ests locally (or, really, having my agent run the relevant tests locally). My agent decided to only run 22 out of the 23 relevant tests because one of the makes some kind of persistent change (if I were doing it myself, I would've reset the state at the start each time to run every test).

Variant	Runs	Mean	Median
Current trunk helper	113s, 114s, 115s	114.0s	114s
PR #77726 helper	83s, 80s, 78s	80.3s	80s

Local speedup for this affected slice: ~34s per branch run.

The main signal is in the Site Editor Loading iterations:

Metric	Current trunk	With PR
firstBlock q50	~5305 ms	~2269 ms

Apologies for the pointless mean and median in the AI generated table.

I think this happens twice per CI run, so that's 68s when run locally on the laptop I used (M3 Max, lower core config for the M3 Max). I don't have a full CI job run time for this machine (I did it on another machine at one point, but 100% of the memory on that machine is being used for various fuzz tests at the moment). In any case, I doubt the 23rd test reverses this (in all likelihood, it increases the delta) and from a performance / gain standpoint, I feel pretty comfortable with the idea that this speeds up the CI job (from a correctness standpoint, this is AI written code in a codebase I know nothing about, I'm less confident there).

danluu · 2026-05-04T19:32:27Z

Alright, here are the results with (allegedly) all relevant tests:

Variant	Runs	Mean
Current trunk helper	123s, 126s, 126s	125.0s
PR #77726 helper	88s, 88s, 90s	88.7s

Metric	Current trunk helper	PR #77726 helper
firstBlock q50	~5287 ms	~2256 ms

Also, looking at the Performance Tests job run time, it seems to have improved over when this PR was created for some reason (possibly just random variance):

Merged PR	Run / job	Merged	Run performance tests duration
#77126 (#77126)	job 74271531357 (https://github.com/WordPress/gutenberg/actions/runs/25333126940/job/74271531357)	2026-05-04	38m 48s
#77862 (#77862)	job 74157108929 (https://github.com/WordPress/gutenberg/actions/runs/25296899823/job/74157108929)	2026-05-04	37m 53s
#77807 (#77807)	job 74256255937 (https://github.com/WordPress/gutenberg/actions/runs/25328686867/job/74256255937)	2026-05-04	40m 39s
#77848 (#77848)	job 74050253259 (https://github.com/WordPress/gutenberg/actions/runs/25253961017/job/74050253259)	2026-05-02	37m 58s
#76318 (#76318)	job 66428556648 (https://github.com/WordPress/gutenberg/actions/runs/22895526355/job/66428556648)	2026-03-10	39m 38s

Unfortunately, we don't seem to break out how long each part takes (yet?).

jsnajdr · 2026-05-05T17:37:06Z

If we want reliable and fast code to detect if the site editor has finished loading, we already have it inside the useIsSiteEditorLoading hook. The source info for this detection is data fetching stats in the core data store: whether any selectors are currently resolving, and how many have been already resolved. In common language this means how many network fetches are currently running and how many have finished.

The useIsSiteEditorLoading hook has all the logic we need, including timeouts:

The isLoading state is initially true.
If it has been true for 10 seconds, it becomes false no matter what. I.e., there is a hard 10s timeout.
If there are any selectors currently resolving, the isLoading state continues to be true.
If no selectors are currently resolving, start waiting for 100ms. This covers short pauses.
If no selectors have been resolving for more than 100ms, set isLoading to false.

Exactly the same logic should be used by e2e tests, there is no need to do something different, like a 60s timeout. We should move the logic to something that is not a React hook, which is usable only inside a React component context. For example, to a store selector that we can call with page.waitForFunction:

await page.waitForFunction( () => ! window.wp.data.select( 'core/site-editor' ).isSiteEditorLoading() );

This should be the one-liner that does all the waiting we need. And it looks directly at the site editor internal state, doesn't depend on catching the DOM elements with loaders.

I'll try to create an alternative PR that implements this idea, I'm curious how the results change. Both in reliability and timing.

github-actions Bot added [Package] E2E Tests /packages/e2e-tests First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository labels Apr 28, 2026

danluu changed the title ~~Avoid missed site editor canvas loader wait~~ Fix wait / sleep which is causing many PRs to fail performance tests in CI for reasons unrelated to the PR Apr 28, 2026

danluu closed this Apr 28, 2026

danluu reopened this Apr 28, 2026

danluu force-pushed the try/perf-test-speedup-5 branch from 0cf755b to e51d754 Compare April 28, 2026 01:42

danluu marked this pull request as ready for review April 28, 2026 01:43

danluu requested a review from kevin940726 as a code owner April 28, 2026 01:43

danluu force-pushed the try/perf-test-speedup-5 branch from e51d754 to 72498c6 Compare April 28, 2026 02:26

dmsnell reviewed Apr 28, 2026

View reviewed changes

danluu mentioned this pull request Apr 28, 2026

RTC: Fuzz testing #77716

Open

dmsnell reviewed Apr 28, 2026

View reviewed changes

Comment thread packages/e2e-test-utils-playwright/src/admin/visit-site-editor.ts Outdated

dmsnell mentioned this pull request May 3, 2026

WIP: Measure impact of perf test change in #77726 #77895

Draft

danluu mentioned this pull request May 4, 2026

Keyboard delays in Performance Tests CI test are unrealistic and cause unnecessarily long CI job runs due to 1s keypress delay #77896

Open

danluu and others added 3 commits May 4, 2026 19:07

Avoid missed site editor canvas loader wait

33fe177

Refactor site editor canvas wait predicate

ed04060

Rearrange types, adjust helper function, move selectors into helper.

0a1bf7d

danluu force-pushed the try/perf-test-speedup-5 branch from 796b338 to 0a1bf7d Compare May 5, 2026 02:08

danluu added 2 commits May 4, 2026 21:20

Force CI rerun

6d3c880

Force Performance CI rerun

ce6d156

jsnajdr mentioned this pull request May 6, 2026

Site Editor e2e tests: reimplement the wait for load #77981

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix wait / sleep which is causing many PRs to fail performance tests in CI for reasons unrelated to the PR#77726

Fix wait / sleep which is causing many PRs to fail performance tests in CI for reasons unrelated to the PR#77726
danluu wants to merge 5 commits intoWordPress:trunkfrom
danluu:try/perf-test-speedup-5

danluu commented Apr 28, 2026 •

edited by dmsnell

Loading

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

github-actions Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

dmsnell left a comment

Uh oh!

Uh oh!

dmsnell commented May 4, 2026

Uh oh!

danluu commented May 4, 2026

Uh oh!

dmsnell commented May 4, 2026

Uh oh!

danluu commented May 4, 2026

Uh oh!

danluu commented May 4, 2026 •

edited

Loading

Uh oh!

danluu commented May 4, 2026 •

edited

Loading

Uh oh!

jsnajdr commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

danluu commented Apr 28, 2026 • edited by dmsnell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

BEGIN AI GENERATED TEXT

Relevant PRs and commits

What the helper does

Evidence from CI

Local reproduction and double-check

Why #77443 is the trigger but not the actual slowdown

Fix direction

END AI GENERATED TEXT

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmsnell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmsnell commented May 4, 2026

Uh oh!

danluu commented May 4, 2026

Uh oh!

dmsnell commented May 4, 2026

Uh oh!

danluu commented May 4, 2026

Uh oh!

danluu commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danluu commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsnajdr commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danluu commented Apr 28, 2026 •

edited by dmsnell

Loading

github-actions Bot commented Apr 28, 2026 •

edited

Loading

danluu commented May 4, 2026 •

edited

Loading

danluu commented May 4, 2026 •

edited

Loading