Enhanced Playwright tests - #4581
Conversation
|
💂♂️ PR Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves Playwright E2E test reliability by replacing brittle waits/selectors with retryable assertions and more stable element/media selection, and by tuning Playwright runtime configuration for consistent repro locally/CI.
Changes:
- Replace fixed sleeps and “count then assert” patterns with locator-based auto-waiting assertions (
toHaveCount,waitFor). - Make tests environment-stable by selecting media/menu items via REST API or label text instead of DB-dependent IDs/grid order.
- Adjust Playwright config and CI artifact upload behavior (configurable workers/retries, timeouts, trace artifact naming).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e-tests/utils.ts | Use retryable locator assertions and avoid repeated count() calls inside loops. |
| e2e-tests/specs/customizer/typography/font-family.spec.ts | Switch to locator assertions and guard against empty heading selections. |
| e2e-tests/specs/customizer/style-book/style-book.spec.ts | Replace fixed timeout with explicit wait for injected Style Book button. |
| e2e-tests/specs/customizer/scroll-to-top/scroll-to-top.spec.ts | Select media via REST API and assert against the selected attachment URL. |
| e2e-tests/specs/customizer/hfg/hfg-menu-item-alignment.spec.ts | Replace DB-dependent menu-item selector with label-based locator filtering. |
| e2e-tests/specs/customizer/hfg/hfg-logo-component.spec.ts | Replace media grid traversal with REST API query for stable attachment picking. |
| e2e-tests/specs/customizer/general/custom-global-colors.spec.ts | Ensure deterministic editor state before toggling palette colors; wait for attribute updates. |
| e2e-tests/specs/admin/tpc-notice-install.spec.ts | Add setup to ensure plugin is removed and notice state is reset per test run. |
| e2e-tests/specs/accessibility/aria.spec.ts | Avoid mis-click navigation by dispatching click to overlay instead of forced click. |
| e2e-tests/playwright.config.ts | Add env-driven worker/retry settings; set action/navigation timeouts. |
| .github/workflows/playwright.yml | Improve trace artifact naming and ignore missing artifacts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
e2e-tests/specs/customizer/general/custom-global-colors.spec.ts:41
- Both the waitForFunction and evaluate blocks dereference window.wp/data/select immediately. If the editor stores are not ready yet, this can throw inside the page context and fail the test instead of polling until ready. Make the predicate resilient (e.g., optional chaining + returning false until the store/settings exist), and similarly guard the evaluate path so it no-ops until the first block is available (or wait explicitly for blocks before updating attributes).
await page.waitForFunction(() =>
(
window.wp.data.select('core/block-editor').getSettings()
.colors || []
).some((color: { slug: string }) => color.slug === 'custom-1')
);
e2e-tests/specs/customizer/general/custom-global-colors.spec.ts:52
- Both the waitForFunction and evaluate blocks dereference window.wp/data/select immediately. If the editor stores are not ready yet, this can throw inside the page context and fail the test instead of polling until ready. Make the predicate resilient (e.g., optional chaining + returning false until the store/settings exist), and similarly guard the evaluate path so it no-ops until the first block is available (or wait explicitly for blocks before updating attributes).
await page.evaluate(() => {
const { data } = window.wp;
const [block] = data.select('core/block-editor').getBlocks();
data.dispatch('core/block-editor').updateBlockAttributes(
block.clientId,
{ backgroundColor: 'custom-1' }
);
});
e2e-tests/utils.ts:309
- Using
not.toHaveCount(0)ensures the container isn't empty, but it doesn't validate that the number of children matchesexpectedOrder, and it also prevents intentional assertions for empty containers. A more robust pattern is to asserttoHaveCount(expectedOrder.length)up front and then iterate using that expected length; this yields clearer failures when the DOM has extra/missing items.
const elements = page.locator(containerSelector + ' > *');
// Without this the loop below silently passes on an empty container.
await expect(elements).not.toHaveCount(0);
const count = await elements.count();
for (let i = 0; i < count; i++) {
await expect(elements.nth(i)).toHaveClass(
new RegExp(`${expectedOrder[i]}`)
);
e2e-tests/playwright.config.ts:15
- The inline comment says the timeout defaults to 100 seconds, but 150_000ms is 150 seconds. Update the comment to match the value (or adjust the fallback value if 100 seconds is intended) to avoid confusion when tuning CI timeouts.
timeout: envInt('TIMEOUT', 150_000), // Defaults to 100 seconds.
e2e-tests/specs/customizer/style-book/style-book.spec.ts:11
- The sentence has a grammatical issue: add punctuation or change 'Booting' to continue the sentence correctly (e.g., use a period before the next sentence or change to 'booting').
// The controls bundle injects this button at runtime, Booting the
// customizer plus its React bundle is the slowest step in the suite and can
// pass the default action timeout on a loaded machine, hence the override.
| const endpoint = `${baseURL}/wp-json/wp/v2/plugins/${TPC_PLUGIN}`; | ||
|
|
||
| await request | ||
| .put(endpoint, { data: { status: 'inactive' } }) | ||
| .catch(() => null); | ||
| await request.delete(endpoint).catch(() => null); |
There was a problem hiding this comment.
WordPress's plugin route is (?P<plugin>[^.\/]+(?:\/[^.\/]+)?); it deliberately permits exactly one slash because plugin identifiers are dir/file. The current code works, and encodeURIComponent would produce the 404 it warns about.
Summary
Enhanced the reliability and maintainability of the Playwright end-to-end (E2E) test suite by improving test selectors, stabilizing test data access, and making test configuration more flexible and robust.