Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: 0 B Total Size: 1.73 MB ℹ️ View Unchanged
|
wp/6.6 branch Actions: Stabilize site-editor tests and e2e test infrastructure
53e841f to
d6d9865
Compare
d6d9865 to
2f36c4c
Compare
Co-authored-by: WunderBart <bartkalisz@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: GraemeF <graemef@git.wordpress.org>
…sting DataViews keyboard nav. Ported from 234d95b94 (#82117), extracting only these two stabilization waits from a much larger commit that otherwise adds unrelated extensible-site-editor-v2 support not applicable to this branch. The iframe wait in particular directly targets the observed failure: the list layout mounts a preview iframe (editor-canvas) for the selected item, and its load can steal focus/rendering mid-test if not waited for.
2f36c4c to
f7f303d
Compare
The DataViews list layout tests start each keyboard sequence by clicking
the Pages search box. On this branch the list layout hides each row's
action buttons with `opacity: 0; position: fixed` inside the scrollable
`.dataviews-wrapper`, and once Ariakit activates the first row Chromium
can be left with stale paint and hit-testing: the page renders blank and
every click lands on `<html>` ("<html> intercepts pointer events"),
even though the DOM is correct. A one pixel viewport resize restores it,
which confirms the DOM is not at fault.
Upstream fixed the underlying styles in #63299, after
this branch was cut. Rather than change shipped styles on a frozen
branch, move focus into the search box with `focus()`. The tests only
use the search box as the starting point of a keyboard sequence, and
focus and key presses do not depend on hit-testing.
Measured locally against this stack with Playwright 1.49.1 and no
retries: the unmodified spec failed 14 of 15 runs; with this change it
passed 49 of 50, and 29 of 30 tests that started with the page in the
broken state still passed. The remaining failure is a separate ordering
flake between the two pages the spec creates.
The DataViews list layout tests expect exactly two pages, listed as "Privacy Policy" then "Sample Page", and the Pages list sorts by date, newest first. The pages were created back to back without a date, so their order depended on timing. When both land in the same second the dates tie and the lower post ID, "Privacy Policy", comes first. When the two requests straddle a second boundary, "Sample Page" is newer and comes first, and every test in the file fails. Delete any existing pages first, then give each page an explicit date with "Privacy Policy" the newer of the two. Deleting first matters: with fixed past dates, a page left behind by another spec would otherwise sort above both. Each CI shard runs one worker against its own WordPress instance, so nothing else is using the pages while this runs. Measured locally against this stack with no retries. Forcing a 1.1s gap between the two creations failed all 15 runs before this change and passed all 15 after. A leftover page from another spec failed all 15 runs before, still failed all 15 with the dates but without the delete, and passed all 15 with both. The spec passed 50 of 50 runs on its own and all its tests passed in a full run of the site editor specs.
What?
RequestUtils.createPage()to pass payloads usingdatainstead ofparams.listreporter (previously suppressed in CI).dataviews-list-layout-keyboard.spec.js'sbeforeEach— for Ariakit's active-item and for the editor-canvas preview iframe to mount.focus()instead ofclick()at the start of eachdataviews-list-layout-keyboard.spec.jstest.Why?
Playwright's
request.fetch()always serializes parameters as URL query parameters regardless of the HTTP method being used. There are some E2E tests that are submitting data large enough to trigger nginx's default request-line length limit, so the request never reaches PHP and encounters a 414 Request-URI Too Long HTML page, which the REST client'sresponse.json()call then fails to parse.See Performance Tests: Update method for creating sample content #69210.
wp-env's port can open before its REST API is actually routable, causing the discovery call to fail almost instantly after test-run startup with no visible error — CI's Playwright config was also suppressing the verbose reporter, silently swallowing the real error when this happened. Ported from WordPress/gutenberg#62331.The Pages DataViews list mounts a preview iframe for the selected item and activates its first item asynchronously; sending keyboard/click input before both settle causes intermittent
TimeoutErrors. Ported — only the two stabilization waits, not the unrelated extensible-site-editor-v2 support bundled in the same upstream commit — fromWordPress/gutenberg@234d95b94(#82117).On this branch the DataViews list layout hides each row's action buttons with
opacity: 0; position: fixedinside the scrollable.dataviews-wrapper. Once Ariakit activates the first row, Chromium can be left with stale paint and hit-testing: the page renders blank and every click lands on<html>(<html> intercepts pointer events), even though the DOM is correct. That is what failedPlaywright - 5on Repairwp/6.6branch Actions: Improve reliability of TinyMCE-related tests #83465. Upstream fixed the underlying styles in WordPress/gutenberg#63299 after this branch was cut; rather than change shipped styles on a frozen branch, the tests now focus the search box, since they only use it as the start of a keyboard sequence and focus and key presses don't depend on hit-testing.Measured locally against this stack with Playwright 1.49.1 and no retries: the unmodified spec failed 14 of 15 runs; with this change it passed 49 of 50, and 29 of 30 tests that started with the page in the broken state still passed. The remaining failure was the separate ordering flake fixed by item 5.
The spec expects exactly two pages, listed as "Privacy Policy" then "Sample Page", and the Pages list sorts by date, newest first. The pages were created back to back without a date: when both land in the same second the dates tie and the lower post ID ("Privacy Policy") comes first, but when the two requests straddle a second boundary "Sample Page" is newer and every test in the file fails. Existing pages are deleted first because, with fixed past dates, a page left behind by another spec would otherwise sort above both; each CI shard runs one worker against its own WordPress instance.
Measured locally with no retries: forcing a 1.1s gap between the two creations failed 15 of 15 runs before and passed 15 of 15 after; a leftover page from another spec failed 15 of 15 before, still failed 15 of 15 with the dates but without the delete, and passed 15 of 15 with both. The spec passed 50 of 50 runs on its own, and all its tests passed in a full run of the site editor specs against a freshly reset database.
Use of AI Tools
This PR was created by Claude Code under my supervision. All code should be treated as AI-produced and not yet reviewed by a human until this PR is marked Ready for Review.