Perf tests: fix formsResponses scenario broken by the wp-build boot layout change (#49272) - #50492
Merged
LiamSarsfield merged 4 commits intoJul 16, 2026
Conversation
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
github-actions
Bot
added
[Status] Needs Author Reply
LiamSarsfield
force-pushed
the
forms-729-fix-the-formsresponses-codevitals-scenario-broken-by-the
branch
from
July 14, 2026 13:24
fe188a3 to
d4b0643
Compare
…ayout change (#49272) The formsResponses CodeVitals scenario has failed every iteration since #49272, freezing the forms performance trend. The rebuilt wp-build `boot` shell now renders the dashboard inside a `position: absolute` `.boot-layout` element under `display: contents` wrappers, so the outer `#jetpack-forms-responses-wp-admin-app.boot-layout-container` mount point — the exact selector measure-lcp.js waited on — computes to height 0. Playwright's visible-state wait never resolves for a 0-height element, so the run timed out ("locator resolved to hidden") even though the dashboard renders correctly. - scenarios.js: wait on the rendered `.boot-layout` surface instead of the 0-height mount point. - measure-lcp.js: add a per-scenario `loadState` (forms uses `load`) so the measurement no longer hangs on the dashboard's canUser OPTIONS probe to /wp/v2/settings, whose response is intermittently not delivered in the headless-Chromium fixture; a single perpetually-pending request can no longer blackhole the scenario. Readiness is carried by the visible selector + hydration + a resource-count settle that a stuck request cannot stall. - Tests for the resilient settle and the forms readiness config. Verified locally (3 iterations, --skip-codevitals): formsResponses green (LCP 252 / TTFB 46 / FCP 72 / decodedBytesKB 6574, 91 resources — all within SANITY_RANGES); jetpackConnected and myJetpack unregressed.
Review follow-up for FORMS-729. The resource-count settle introduced for the formsResponses scenario could cap out (or settle during a mid-load plateau) and still capture the page, posting an undercounted decodedBytesKB to the append-only CodeVitals store past a loose minResourceCount floor. - waitForResourceCountIdle now reports settled-vs-capped, and measureLCP fails the iteration (fail closed) when the count was still changing at the deadline, at both settle sites. - The initial navigation also settles before the measured reload on the non-networkidle path, so the reload measures a warm cache like the networkidle scenarios always did. - formsResponses minResourceCount raised 40 -> 64 (~70% of the observed 91-resource load, matching myJetpack), with an exact-pin + boundary test. - Settle unit tests now assert the stability algorithm (poll counts, streak reset, cap reporting) instead of resolve-only. - Docs: README forms note corrected (the /wp/v2/settings stall is a local browser-side delivery stall, not a server hang), and loadState is now documented in the measureLCP docblock and the scenarios guide.
Review follow-up for FORMS-729. Both independent review passes converged on the same finding, one with a live repro: a slow legitimate response is invisible to the completed-resource count (a Resource Timing entry appears only at responseEnd), so the settle could report quiet and capture before that response landed, posting an undercounted decodedBytesKB. - New trackPendingRequests ledger: counts requests from issue to finished/failed, excluding only the known-stuck /wp/v2/settings OPTIONS probe (isStuckSettingsProbe). waitForResourceCountIdle now also requires zero relevant in-flight requests on every stable poll — networkidle's quiet + nothing-in-flight guarantee minus the one request that breaks it. Any OTHER stuck request now holds the settle open and fails the iteration at the deadline instead of being silently ignored. - Install the LCP/resource-timing init script before the first navigation so the warm-up settle reads an uncapped Resource Timing buffer (the browser's 250-entry default would silently cap the count once the load grows). - Comments and docs stop overclaiming: the minResourceCount floor only catches captures below its value; the residual quiet-gap window (shared with networkidle itself) is documented and pinned by a dedicated test. - README: the Capture-guards floor now defers to scenarios.js instead of restating numbers that had already drifted (still said 40/~80). - New tests: in-flight request holds the settle open, a never-clearing request fails closed, the isStuckSettingsProbe match matrix, ledger add/remove/exclude/dispose, and the accepted-residual documenting test.
…seam Round-4 review follow-up for FORMS-729 (both review passes; the over-match was empirically driven through the settle with a held-open near-miss route). - isStuckSettingsProbe now compares the decoded REST route exactly (rest_route query value or /wp-json pathname, modulo a trailing slash) instead of a substring match, so adjacent routes (/wp/v2/settings/child, /wp/v2/settings-extra) and URLs merely carrying the string in a query value stay in the ledger and fail the iteration if they get stuck — matching the documented 'only that one probe' contract. Near-miss cases added to the matcher test. - New settleOrThrow seam used by both readiness sites, with a test pinning that the in-flight ledger is threaded into the settle — the exact one-line mutation the review proved could silently revert the in-flight guarantee with the whole suite green now fails a test. The settle-path predicate is bound once (useResourceSettle) instead of re-derived at three sites. - The fail-closed error message no longer claims the count was 'still changing' when a stuck in-flight request may be the actual holdout (settled:false cannot distinguish the two signals). - Comments/README stop calling SANITY_RANGES a backstop for the quiet-gap residual (the range is far too wide to catch an undercount); the settle's ~1s-quiet requirement is named as the working defense.
LiamSarsfield
marked this pull request as ready for review
July 14, 2026 21:16
LiamSarsfield
force-pushed
the
forms-729-fix-the-formsresponses-codevitals-scenario-broken-by-the
branch
from
July 14, 2026 21:16
3d94538 to
346f2b5
Compare
LiamSarsfield
deleted the
forms-729-fix-the-formsresponses-codevitals-scenario-broken-by-the
branch
July 16, 2026 08:26
github-actions
Bot
removed
[Status] Needs Author Reply
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.
Fixes FORMS-729: https://linear.app/a8c/issue/FORMS-729
Proposed changes
The formsResponses CodeVitals scenario has failed every run since #49272, which left the forms performance trend dark. This fixes the scenario and makes the measurement fail loud instead of posting bad numbers the next time the page changes under us.
Root cause: #49272 rebuilt the wp-build boot shell so the dashboard renders inside a
position: absolute.boot-layoutchild. The mount point our selector waited on (#jetpack-forms-responses-wp-admin-app.boot-layout-container) now computes to height 0, and Playwright's visible-state wait never resolves for a 0-height element. The app renders fine. Only the wait was broken. The ticket blamed @WordPress version skew and a null render; reproducing on a fresh fixture ruled both out (full UI on WP 6.9.4 and 7.0, before and after #49272), so #50214 is not the fix either.Changes, all in
tools/performance:.boot-layout(has height) instead of the 0-height mount point.networkidle: the page'scanUserOPTIONS probe to/wp/v2/settingssometimes never completes in the local Docker fixture, so navigation timed out at 60s. Nowloadplus a settle: completed-resource count holds steady with zero requests in flight, excluding only that probe (exact REST route match). Same guarantee asnetworkidle, minus the request that breaks it.decodedBytesKB. Still count-based, so the planned editor lazy-load drop stays unclipped.networkidleuntouched; tests pin all three configs.Known residual: a quiet gap where nothing is in flight and the next resource wave has not started can still settle early.
networkidlehas the same gap at 500ms of quiet; this settle requires 1s. A test documents it.Other information
optional: truethere, so even a forms scenario that fails every iteration skips its own keys while the dashboard still posts.1d9724d: 94 resources, 9066 KB), at (0e32517), and after Update Bundled @wordpress/* monorepo #49272, each on a fresh fixture. The trend will show a real payload drop at Update Bundled @wordpress/* monorepo #49272, from about 9066 KB to 6574 KB.Does this pull request change what data or activity we track or use?
No. Test tooling only; no changelog needed for
tools/changes.Testing instructions
From
tools/performance, with no CodeVitals token set:pnpm test:unitpasses 153 tests.SCENARIO=all node scripts/run-performance-tests.js --skip-codevitalsruns all three scenarios green, 5/5 iterations. formsResponses captures 91 resources / 6574 KB on every iteration; before this PR it timed out on every iteration.