feat: add Playwright e2e tests for the block editor#40
Conversation
Introduce Playwright-based end-to-end testing with a shared CI build job, block editor coverage for the Interactions toolbar and library insert flow, and wp-env tooling for local and matrix testing across supported WP/PHP versions. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a complete Playwright E2E testing setup for the Interactions plugin, including WordPress fixtures, Block Editor and Elementor coverage, custom reporting, local configuration, and a PHP/WordPress GitHub Actions matrix. ChangesPlaywright E2E testing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant WordPressEnv
participant Playwright
participant InteractionsPlugin
GitHubActions->>WordPressEnv: start PHP/WordPress matrix environment
GitHubActions->>Playwright: run npm run test
Playwright->>WordPressEnv: create authenticated request context
Playwright->>InteractionsPlugin: open Block Editor or Elementor interaction UI
InteractionsPlugin-->>Playwright: render sidebar, preset, or interaction list
Playwright-->>GitHubActions: publish test results and failure artifacts
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Verify the Interactions launcher appears in the lower right when editing a page with Elementor, and install/activate Elementor in wp-env and CI. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
e2e/readme.md (2)
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify the language for the code block.
Add the
dotenvorenvlanguage identifier to the fenced code block to improve syntax highlighting and resolve a markdownlint warning.♻️ Proposed refactor
-``` +```env WP_BASE_URL=http://localhost:8889🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/readme.md` around lines 10 - 16, Update the fenced environment-variable block in the README to specify the env or dotenv language identifier, preserving all existing variable entries.Source: Linters/SAST tools
44-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix capitalization of "GitHub".
The official name of the platform is spelled with a capital "H".
♻️ Proposed refactor
- Our main basis: https://github.com/meszarosrob/wordpress-e2e-playwright-intro-2023 -- Gutenberg e2e Github workflow: https://github.com/WordPress/gutenberg/blob/trunk/.github/workflows/end2end-test.yml +- Gutenberg e2e GitHub workflow: https://github.com/WordPress/gutenberg/blob/trunk/.github/workflows/end2end-test.yml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/readme.md` around lines 44 - 45, Update the platform name in the README reference to use the official capitalization “GitHub,” including the workflow description and URL text where applicable.Source: Linters/SAST tools
e2e/test-utils/editor.ts (1)
3-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse primitive
stringinstead of theStringobject wrapper.In TypeScript,
Stringrefers to the JavaScriptStringobject, which behaves differently from the primitivestringtype. Best practice is to use the primitivestringtype for variables and arrays (string[]orArray<string>).♻️ Proposed refactor
class ExtendedEditor extends BaseEditor { - blockErrors: Array<String> = [] + blockErrors: string[] = [] - getBlockAttributes = async function( clientId : String ) { + getBlockAttributes = async function( clientId: string ) { await this.page.waitForFunction( () => window?.wp?.blocks && window?.wp?.data ) const attributes = await this.page.evaluate( async ( [ _clientId ] ) => { return await window.wp.data.select( 'core/block-editor' ).getBlockAttributes( _clientId ) }, [ clientId ] ) return attributes } - getBlockErrors() :Array<String> { + getBlockErrors(): string[] { this.blockErrors = [] this.page.on( 'console', msg => { if ( msg.type() === 'error' && msg.text().startsWith( 'Block validation' ) ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/test-utils/editor.ts` around lines 3 - 28, Replace the boxed String types with the primitive string type in ExtendedEditor, including blockErrors, the clientId parameter, and the getBlockErrors return type; preserve the existing behavior.e2e/config/global-setup.ts (1)
26-26: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against a missing
INTERACTIONS_SLUG.If the env var isn't set, this calls
activatePlugin(undefined), producing a confusing REST error instead of a clear setup failure. Since this runs once before the whole suite, a clear message saves debugging time for anyone missing the env var locally.Suggested fix
+ if ( ! process.env.INTERACTIONS_SLUG ) { + throw new Error( 'INTERACTIONS_SLUG environment variable is required for global setup.' ) + } + await requestUtils.activatePlugin( process.env.INTERACTIONS_SLUG )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/config/global-setup.ts` at line 26, Validate that INTERACTIONS_SLUG is set before the activatePlugin call in global setup, and fail immediately with a clear configuration error when it is missing. Only invoke requestUtils.activatePlugin with a defined slug.tools/playwright-test-matrix.js (2)
78-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the version-arithmetic helpers.
subtractMinorVersions,getMinorKey,resolveLatestWordPressPatch, andbuildPlaywrightMatrixencode non-obvious invariants (e.g., minor-version wraparound assumes exactly 10 minors per major) and are already exported (lines 223-239) specifically for testability, but no spec accompanies this file in the reviewed diff. Given they directly determine the CI test matrix, cheap unit tests would catch version-math regressions before they silently produce a wrong matrix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/playwright-test-matrix.js` around lines 78 - 138, Add unit tests for the exported helpers subtractMinorVersions, getMinorKey, resolveLatestWordPressPatch, and buildPlaywrightMatrix. Cover minor-version subtraction and wraparound, minor-key normalization, latest patch selection with no-match fallback, and the generated matrix entries for supported WordPress and PHP versions.
203-214: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant duplicate network call to the WordPress version-check API.
fetchLatestWordPressRelease()(lines 73-76) already callsfetchWordPressReleases()internally, butmain()doesn't pass the resulting list intosyncPlaywrightTestMatrix, sosyncPlaywrightTestMatrix(line 173) fetches it a second time. This doubles network round-trips and doubles the chance of a transient failure for no benefit.Suggested fix
const fetchLatestWordPressRelease = async () => { const releases = await fetchWordPressReleases() - return releases[ 0 ] + return { latest: releases[ 0 ], releases } }const main = async () => { const testedUpTo = getTestedUpToFromReadme() if ( ! testedUpTo ) { throw new Error( 'Could not read "Tested up to" from readme.txt' ) } - const latestRelease = await fetchLatestWordPressRelease() + const { latest: latestRelease, releases } = await fetchLatestWordPressRelease() await syncPlaywrightTestMatrix( { testedUpTo, minPhp: latestRelease.phpVersion, + releases, } ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/playwright-test-matrix.js` around lines 203 - 214, Reuse the WordPress releases fetched by fetchLatestWordPressRelease() when calling syncPlaywrightTestMatrix in main, so the matrix sync does not invoke fetchWordPressReleases() again. Update the relevant function signatures and call sites to pass the existing release list while preserving the current minPhp and testedUpTo behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/playwright.yml:
- Line 11: Update both actions/checkout@v4 steps in the build and test jobs to
disable credential persistence by setting persist-credentials to false,
preserving the existing checkout behavior otherwise.
- Around line 1-9: Add an explicit least-privilege permissions block at the
workflow or build job level in the Playwright Tests workflow, granting only the
read access required for checkout and the write access required for artifact or
summary uploads; set all unspecified permissions to none.
- Around line 129-133: Update the “Add test results to summary” step to pass
steps.mto.outputs.body and steps.artifact-upload-step.outputs.artifact-id
through the step’s env context, then reference only those environment variables
inside run. Preserve the existing summary text and artifact URL while preventing
untrusted output from being interpolated into shell script syntax.
In `@e2e/config/reporter.ts`:
- Line 7: Import and use the exported ansiRegex in the reporter’s error
formatting flow to remove ANSI escape sequences from Playwright error.message
and error.snippet before generating Markdown or GitHub step-summary output.
Preserve the existing report content apart from this sanitization, and reuse the
ansiRegex export from e2e/test-utils/format.ts rather than adding a new pattern.
- Around line 58-67: Update the error-reporting loop in reporter handling to
apply the imported ansiRegex when appending each error.message and
error.snippet, ensuring ANSI escape codes are removed before writing the
Markdown testResult while preserving the existing formatting and conditional
checks.
In `@e2e/playwright.config.ts`:
- Around line 10-12: Update the Playwright configuration’s globalSetup path to
use path.resolve with the existing __dirname instead of fileURLToPath and new
URL, and remove the now-unused url module import. Preserve the resolved
global-setup.ts target while ensuring Windows-compatible path handling.
In `@e2e/tests/block-editor.spec.ts`:
- Around line 41-71: Align the enclosing timeout in this interaction-library
test with the cumulative waits used by the assertions, or reduce the individual
assertion timeouts so their worst-case duration fits within test.setTimeout.
Update the timeout configuration around the interaction insertion flow while
preserving the existing diagnostic assertions and their ordering.
---
Nitpick comments:
In `@e2e/config/global-setup.ts`:
- Line 26: Validate that INTERACTIONS_SLUG is set before the activatePlugin call
in global setup, and fail immediately with a clear configuration error when it
is missing. Only invoke requestUtils.activatePlugin with a defined slug.
In `@e2e/readme.md`:
- Around line 10-16: Update the fenced environment-variable block in the README
to specify the env or dotenv language identifier, preserving all existing
variable entries.
- Around line 44-45: Update the platform name in the README reference to use the
official capitalization “GitHub,” including the workflow description and URL
text where applicable.
In `@e2e/test-utils/editor.ts`:
- Around line 3-28: Replace the boxed String types with the primitive string
type in ExtendedEditor, including blockErrors, the clientId parameter, and the
getBlockErrors return type; preserve the existing behavior.
In `@tools/playwright-test-matrix.js`:
- Around line 78-138: Add unit tests for the exported helpers
subtractMinorVersions, getMinorKey, resolveLatestWordPressPatch, and
buildPlaywrightMatrix. Cover minor-version subtraction and wraparound, minor-key
normalization, latest patch selection with no-match fallback, and the generated
matrix entries for supported WordPress and PHP versions.
- Around line 203-214: Reuse the WordPress releases fetched by
fetchLatestWordPressRelease() when calling syncPlaywrightTestMatrix in main, so
the matrix sync does not invoke fetchWordPressReleases() again. Update the
relevant function signatures and call sites to pass the existing release list
while preserving the current minPhp and testedUpTo behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfa7fec0-3e0d-4c98-8edb-fc95d6bd9129
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
.env.example.github/workflows/playwright.yml.gitignore.wp-env.jsone2e/config/global-setup.tse2e/config/reporter.tse2e/playwright.config.tse2e/readme.mde2e/test-utils/editor.tse2e/test-utils/format.tse2e/test-utils/index.tse2e/test-utils/interactions.tse2e/test-utils/requestUtils.tse2e/test-utils/test.tse2e/tests/block-editor.spec.tspackage.jsontools/playwright-test-matrix.jstsconfig.json
| Reporter, Suite, TestCase, | ||
| } from '@playwright/test/reporter' | ||
|
|
||
| import { ms } from 'e2e/test-utils' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Import ansiRegex to strip color codes.
Playwright's error.message and error.snippet include ANSI escape codes for terminal coloring. If these aren't stripped, they will pollute the resulting Markdown report (e.g., showing up as [31mExpected[39m) and render poorly in GitHub step summaries.
You can utilize the ansiRegex that was exported in e2e/test-utils/format.ts but left unused.
🐛 Proposed fix to sanitize error outputs
-import { ms } from 'e2e/test-utils'
+import { ms, ansiRegex } from 'e2e/test-utils'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ms } from 'e2e/test-utils' | |
| import { ms, ansiRegex } from 'e2e/test-utils' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/config/reporter.ts` at line 7, Import and use the exported ansiRegex in
the reporter’s error formatting flow to remove ANSI escape sequences from
Playwright error.message and error.snippet before generating Markdown or GitHub
step-summary output. Preserve the existing report content apart from this
sanitization, and reuse the ansiRegex export from e2e/test-utils/format.ts
rather than adding a new pattern.
| if ( result.errors.length >= 1 ) { | ||
| result.errors.forEach( error => { | ||
| if ( error.message ) { | ||
| testResult += `${ error.message }` + '\n\n' | ||
| } | ||
|
|
||
| if ( error.snippet ) { | ||
| testResult += `${ error.snippet }` + `\n\n` | ||
| } | ||
| } ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Strip ANSI codes from error messages and snippets.
Applying the imported ansiRegex will sanitize the error details before they are written to the Markdown report.
♻️ Proposed refactor
if ( result.errors.length >= 1 ) {
result.errors.forEach( error => {
if ( error.message ) {
- testResult += `${ error.message }` + '\n\n'
+ testResult += `${ error.message.replace( ansiRegex, '' ) }` + '\n\n'
}
if ( error.snippet ) {
- testResult += `${ error.snippet }` + `\n\n`
+ testResult += `${ error.snippet.replace( ansiRegex, '' ) }` + `\n\n`
}
} )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( result.errors.length >= 1 ) { | |
| result.errors.forEach( error => { | |
| if ( error.message ) { | |
| testResult += `${ error.message }` + '\n\n' | |
| } | |
| if ( error.snippet ) { | |
| testResult += `${ error.snippet }` + `\n\n` | |
| } | |
| } ) | |
| if ( result.errors.length >= 1 ) { | |
| result.errors.forEach( error => { | |
| if ( error.message ) { | |
| testResult += `${ error.message.replace( ansiRegex, '' ) }` + '\n\n' | |
| } | |
| if ( error.snippet ) { | |
| testResult += `${ error.snippet.replace( ansiRegex, '' ) }` + `\n\n` | |
| } | |
| } ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/config/reporter.ts` around lines 58 - 67, Update the error-reporting loop
in reporter handling to apply the imported ansiRegex when appending each
error.message and error.snippet, ensuring ANSI escape codes are removed before
writing the Markdown testResult while preserving the existing formatting and
conditional checks.
| globalSetup: fileURLToPath( | ||
| new URL( './config/global-setup.ts', 'file:' + __filename ).href | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Simplify file path resolution to prevent cross-platform crashes.
The use of fileURLToPath and new URL with 'file:' + __filename will throw a TypeError: Invalid URL on Windows, as drive letters (e.g., C:\) do not form a valid file:// base URL structure without additional parsing.
Since __dirname is successfully used on line 7, you can safely simplify this to use path.resolve, which also allows you to remove the url module import at the top of the file.
♻️ Proposed fix
- globalSetup: fileURLToPath(
- new URL( './config/global-setup.ts', 'file:' + __filename ).href
- ),
+ globalSetup: path.resolve( __dirname, './config/global-setup.ts' ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| globalSetup: fileURLToPath( | |
| new URL( './config/global-setup.ts', 'file:' + __filename ).href | |
| ), | |
| globalSetup: path.resolve( __dirname, './config/global-setup.ts' ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/playwright.config.ts` around lines 10 - 12, Update the Playwright
configuration’s globalSetup path to use path.resolve with the existing __dirname
instead of fileURLToPath and new URL, and remove the now-unused url module
import. Preserve the resolved global-setup.ts target while ensuring
Windows-compatible path handling.
| test.setTimeout( 60000 ) | ||
|
|
||
| const initialBlockCount = await editor.canvas.locator( '[data-type]' ).count() | ||
|
|
||
| // Open the Interactions sidebar first so the app is mounted before the insert event fires. | ||
| await interactions.getSidebarToolbarButton().click() | ||
|
|
||
| // Open the Interaction Library from the top toolbar. | ||
| await interactions.getLibraryToolbarButton().click() | ||
| await expect( interactions.getInteractionLibraryModal() ).toBeVisible() | ||
|
|
||
| // Wait for presets to finish loading. | ||
| const firstFreePreset = interactions.getFirstFreePresetCard() | ||
| await expect( firstFreePreset ).toBeVisible( { timeout: 30000 } ) | ||
|
|
||
| // Hover to reveal the Insert button, then click it. | ||
| await firstFreePreset.hover() | ||
| const insertButton = firstFreePreset.getByRole( 'button', { name: 'Insert' } ) | ||
| await expect( insertButton ).toBeVisible() | ||
| await insertButton.click() | ||
|
|
||
| // Modal should close after inserting. | ||
| await expect( interactions.getInteractionLibraryModal() ).toBeHidden( { timeout: 15000 } ) | ||
|
|
||
| // New blocks should be added to the editor. | ||
| await expect( editor.canvas.locator( '[data-type]' ) ).not.toHaveCount( initialBlockCount, { timeout: 15000 } ) | ||
| await expect( editor.canvas.locator( '[data-type]' ).count() ).resolves.toBeGreaterThan( initialBlockCount ) | ||
|
|
||
| // Interactions sidebar should show an interaction being edited. | ||
| await expect( page.locator( '.interface-complementary-area' ).filter( { has: interactions.getInteractionsSidebar() } ) ).toBeVisible( { timeout: 15000 } ) | ||
| await expect( page.locator( '.interact-interaction-card' ).first() ).toBeVisible( { timeout: 30000 } ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Nested wait timeouts can exceed the enclosing test timeout.
test.setTimeout(60000) caps the whole test at 60s, but the individual expect(...).toBeVisible/toBeHidden/not.toHaveCount(..., { timeout }) calls at lines 54, 63, 66, 70, and 71 alone sum to 105s of worst-case waiting. Under a slow/loaded CI runner, the test can hit the outer 60s timeout well before the later, more diagnostic assertions run, producing an opaque generic timeout instead of a specific assertion failure.
Suggested fix
- test.setTimeout( 60000 )
+ test.setTimeout( 120000 )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test.setTimeout( 60000 ) | |
| const initialBlockCount = await editor.canvas.locator( '[data-type]' ).count() | |
| // Open the Interactions sidebar first so the app is mounted before the insert event fires. | |
| await interactions.getSidebarToolbarButton().click() | |
| // Open the Interaction Library from the top toolbar. | |
| await interactions.getLibraryToolbarButton().click() | |
| await expect( interactions.getInteractionLibraryModal() ).toBeVisible() | |
| // Wait for presets to finish loading. | |
| const firstFreePreset = interactions.getFirstFreePresetCard() | |
| await expect( firstFreePreset ).toBeVisible( { timeout: 30000 } ) | |
| // Hover to reveal the Insert button, then click it. | |
| await firstFreePreset.hover() | |
| const insertButton = firstFreePreset.getByRole( 'button', { name: 'Insert' } ) | |
| await expect( insertButton ).toBeVisible() | |
| await insertButton.click() | |
| // Modal should close after inserting. | |
| await expect( interactions.getInteractionLibraryModal() ).toBeHidden( { timeout: 15000 } ) | |
| // New blocks should be added to the editor. | |
| await expect( editor.canvas.locator( '[data-type]' ) ).not.toHaveCount( initialBlockCount, { timeout: 15000 } ) | |
| await expect( editor.canvas.locator( '[data-type]' ).count() ).resolves.toBeGreaterThan( initialBlockCount ) | |
| // Interactions sidebar should show an interaction being edited. | |
| await expect( page.locator( '.interface-complementary-area' ).filter( { has: interactions.getInteractionsSidebar() } ) ).toBeVisible( { timeout: 15000 } ) | |
| await expect( page.locator( '.interact-interaction-card' ).first() ).toBeVisible( { timeout: 30000 } ) | |
| test.setTimeout( 120000 ) | |
| const initialBlockCount = await editor.canvas.locator( '[data-type]' ).count() | |
| // Open the Interactions sidebar first so the app is mounted before the insert event fires. | |
| await interactions.getSidebarToolbarButton().click() | |
| // Open the Interaction Library from the top toolbar. | |
| await interactions.getLibraryToolbarButton().click() | |
| await expect( interactions.getInteractionLibraryModal() ).toBeVisible() | |
| // Wait for presets to finish loading. | |
| const firstFreePreset = interactions.getFirstFreePresetCard() | |
| await expect( firstFreePreset ).toBeVisible( { timeout: 30000 } ) | |
| // Hover to reveal the Insert button, then click it. | |
| await firstFreePreset.hover() | |
| const insertButton = firstFreePreset.getByRole( 'button', { name: 'Insert' } ) | |
| await expect( insertButton ).toBeVisible() | |
| await insertButton.click() | |
| // Modal should close after inserting. | |
| await expect( interactions.getInteractionLibraryModal() ).toBeHidden( { timeout: 15000 } ) | |
| // New blocks should be added to the editor. | |
| await expect( editor.canvas.locator( '[data-type]' ) ).not.toHaveCount( initialBlockCount, { timeout: 15000 } ) | |
| await expect( editor.canvas.locator( '[data-type]' ).count() ).resolves.toBeGreaterThan( initialBlockCount ) | |
| // Interactions sidebar should show an interaction being edited. | |
| await expect( page.locator( '.interface-complementary-area' ).filter( { has: interactions.getInteractionsSidebar() } ) ).toBeVisible( { timeout: 15000 } ) | |
| await expect( page.locator( '.interact-interaction-card' ).first() ).toBeVisible( { timeout: 30000 } ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/tests/block-editor.spec.ts` around lines 41 - 71, Align the enclosing
timeout in this interaction-library test with the cumulative waits used by the
assertions, or reduce the individual assertion timeouts so their worst-case
duration fits within test.setTimeout. Update the timeout configuration around
the interaction insertion flow while preserving the existing diagnostic
assertions and their ordering.
Extend the Elementor Playwright test to click the launcher and assert the Interactions panel opens with the sidebar visible. Co-authored-by: Cursor <cursoragent@cursor.com>
Use domcontentloaded instead of the load event, wait for Elementor UI markers rather than window.elementor, and use Playwright visibility checks for the Interactions launcher. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
e2e/tests/block-editor.spec.ts (1)
41-41: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNested wait timeouts can exceed the enclosing test timeout.
test.setTimeout( 60000 )caps the whole test at 60s, but the individualexpect(...).toBeVisible/toBeHidden/not.toHaveCount(..., { timeout })calls below sum to 105s of worst-case waiting. Under a slow/loaded CI runner, the test can hit the outer 60s timeout well before the later, more diagnostic assertions run, producing an opaque generic timeout instead of a specific assertion failure.💚 Proposed fix
- test.setTimeout( 60000 ) + test.setTimeout( 120000 )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tests/block-editor.spec.ts` at line 41, Update the timeout configuration in the block editor test around test.setTimeout so the enclosing test timeout accommodates the cumulative assertion wait time used by the toBeVisible, toBeHidden, and not.toHaveCount checks, ensuring later diagnostic assertions can execute before the test is terminated.
🧹 Nitpick comments (2)
e2e/tests/elementor.spec.ts (1)
33-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse non-null assertions to prevent strict TypeScript errors.
expect(...).not.toBeNull()is a runtime assertion and does not act as a type guard. IfstrictNullChecksis enabled in yourtsconfig.json, accessing properties onboxorviewportwill trigger compiler errors. Consider using the non-null assertion operator!to proactively satisfy the type checker.♻️ Proposed refactor
expect( box ).not.toBeNull() expect( viewport ).not.toBeNull() - expect( box.x + box.width ).toBeGreaterThan( viewport.width * 0.5 ) - expect( box.y + box.height ).toBeGreaterThan( viewport.height * 0.5 ) + expect( box!.x + box!.width ).toBeGreaterThan( viewport!.width * 0.5 ) + expect( box!.y + box!.height ).toBeGreaterThan( viewport!.height * 0.5 )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tests/elementor.spec.ts` around lines 33 - 36, Update the assertions in the Elementor test so property access on box and viewport uses non-null assertions after their runtime null checks. Preserve the existing geometry expectations and reference the box and viewport variables directly.e2e/test-utils/requestUtils.ts (1)
4-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
setup()largely duplicates the upstreamRequestUtils.setup()implementation.The mkdir/JSON.parse/ENOENT handling here is nearly identical to the base class's own static
setup()in@wordpress/e2e-test-utils-playwright. Since ES class static methods preserve the calling class asthis, simply inheriting the basesetup()(without overriding it) would already returnnew ExtendedRequestUtils(...)correctly — no override may be needed unless the goal is specifically to pass a trimmed{cookies, origins: []}storageState intonewContext(the base class's ownnewContextcall isn't shown here, so it's unclear if it already handles this).Worth confirming whether this override still earns its complexity, or whether it can be reduced to just relying on inherited
setup()(or a thin wrapper), reducing future drift risk if upstream'ssetup()signature changes (there's an open upstream request to extend it withextraHTTPHeaders/httpCredentials).Reference: upstream `RequestUtils.setup()` (gutenberg/packages/e2e-test-utils-playwright)
Confirmed via search that the base implementation performs the same
fs.mkdir+JSON.parse+ ENOENT-check sequence and returnsnew this(...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/test-utils/requestUtils.ts` around lines 4 - 45, Remove the duplicated static setup implementation from ExtendedRequestUtils and rely on the inherited RequestUtils.setup() so the base class remains the single source of truth. Preserve the ExtendedRequestUtils return type through the inherited this binding, and only retain a wrapper if the trimmed storageState passed to newContext is required by this subclass’s behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/test-utils/interactions.ts`:
- Around line 87-121: Remove the late waitForResponse for /dist/editor.js from
openElementorEditor, since it is registered after the editor load and can miss
the request. Keep the existing `#elementor-panel` and `#elementor-preview-iframe`
readiness waits and subsequent onboarding flow unchanged.
In `@e2e/test-utils/requestUtils.ts`:
- Around line 108-148: Update createInteraction so the generated interaction key
combines the current timestamp with an additional unique entropy source, such as
randomness or a unique identifier, ensuring parallel calls cannot produce the
same key while preserving the existing interaction_e2e_ prefix.
---
Duplicate comments:
In `@e2e/tests/block-editor.spec.ts`:
- Line 41: Update the timeout configuration in the block editor test around
test.setTimeout so the enclosing test timeout accommodates the cumulative
assertion wait time used by the toBeVisible, toBeHidden, and not.toHaveCount
checks, ensuring later diagnostic assertions can execute before the test is
terminated.
---
Nitpick comments:
In `@e2e/test-utils/requestUtils.ts`:
- Around line 4-45: Remove the duplicated static setup implementation from
ExtendedRequestUtils and rely on the inherited RequestUtils.setup() so the base
class remains the single source of truth. Preserve the ExtendedRequestUtils
return type through the inherited this binding, and only retain a wrapper if the
trimmed storageState passed to newContext is required by this subclass’s
behavior.
In `@e2e/tests/elementor.spec.ts`:
- Around line 33-36: Update the assertions in the Elementor test so property
access on box and viewport uses non-null assertions after their runtime null
checks. Preserve the existing geometry expectations and reference the box and
viewport variables directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff8e1a68-d200-48bd-b6a8-ab5d36414064
📒 Files selected for processing (9)
.env.example.github/workflows/playwright.yml.wp-env.jsone2e/config/global-setup.tse2e/readme.mde2e/test-utils/interactions.tse2e/test-utils/requestUtils.tse2e/tests/block-editor.spec.tse2e/tests/elementor.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/readme.md
- e2e/config/global-setup.ts
- .wp-env.json
- .github/workflows/playwright.yml
| createInteraction = async function( title = 'E2E Test Interaction' ) { | ||
| const key = `interaction_e2e_${ Date.now() }` | ||
| const data = JSON.stringify( { | ||
| key, | ||
| title, | ||
| active: true, | ||
| type: 'click', | ||
| target: { | ||
| type: 'block', | ||
| value: '', | ||
| }, | ||
| timelines: [ | ||
| { | ||
| loop: false, | ||
| onceOnly: false, | ||
| alternate: false, | ||
| reset: false, | ||
| reverse: false, | ||
| actions: [], | ||
| }, | ||
| ], | ||
| options: {}, | ||
| locations: [ | ||
| [ | ||
| { | ||
| param: 'all', | ||
| operator: '==', | ||
| value: '', | ||
| }, | ||
| ], | ||
| ], | ||
| } ) | ||
|
|
||
| await this.rest( { | ||
| method: 'POST', | ||
| path: '/interact/v1/update_interaction', | ||
| data: { data }, | ||
| } ) | ||
|
|
||
| return key | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Timestamp-only interaction key risks collisions under parallel test workers.
key = interaction_e2e_${Date.now()} has no randomness. Playwright runs test files across multiple parallel workers by default; two createInteraction() calls in the same millisecond would produce identical keys, and if the backend upserts interactions by key, one test's fixture could silently overwrite another's, causing intermittent, hard-to-diagnose flaky failures.
💡 Suggested fix: add entropy to the key
- const key = `interaction_e2e_${ Date.now() }`
+ const key = `interaction_e2e_${ Date.now() }_${ Math.random().toString( 36 ).slice( 2, 8 ) }`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| createInteraction = async function( title = 'E2E Test Interaction' ) { | |
| const key = `interaction_e2e_${ Date.now() }` | |
| const data = JSON.stringify( { | |
| key, | |
| title, | |
| active: true, | |
| type: 'click', | |
| target: { | |
| type: 'block', | |
| value: '', | |
| }, | |
| timelines: [ | |
| { | |
| loop: false, | |
| onceOnly: false, | |
| alternate: false, | |
| reset: false, | |
| reverse: false, | |
| actions: [], | |
| }, | |
| ], | |
| options: {}, | |
| locations: [ | |
| [ | |
| { | |
| param: 'all', | |
| operator: '==', | |
| value: '', | |
| }, | |
| ], | |
| ], | |
| } ) | |
| await this.rest( { | |
| method: 'POST', | |
| path: '/interact/v1/update_interaction', | |
| data: { data }, | |
| } ) | |
| return key | |
| } | |
| createInteraction = async function( title = 'E2E Test Interaction' ) { | |
| const key = `interaction_e2e_${ Date.now() }_${ Math.random().toString( 36 ).slice( 2, 8 ) }` | |
| const data = JSON.stringify( { | |
| key, | |
| title, | |
| active: true, | |
| type: 'click', | |
| target: { | |
| type: 'block', | |
| value: '', | |
| }, | |
| timelines: [ | |
| { | |
| loop: false, | |
| onceOnly: false, | |
| alternate: false, | |
| reset: false, | |
| reverse: false, | |
| actions: [], | |
| }, | |
| ], | |
| options: {}, | |
| locations: [ | |
| [ | |
| { | |
| param: 'all', | |
| operator: '==', | |
| value: '', | |
| }, | |
| ], | |
| ], | |
| } ) | |
| await this.rest( { | |
| method: 'POST', | |
| path: '/interact/v1/update_interaction', | |
| data: { data }, | |
| } ) | |
| return key | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/test-utils/requestUtils.ts` around lines 108 - 148, Update
createInteraction so the generated interaction key combines the current
timestamp with an additional unique entropy source, such as randomness or a
unique identifier, ensuring parallel calls cannot produce the same key while
preserving the existing interaction_e2e_ prefix.
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/playwright.yml:
- Around line 139-142: Update the workflow step that normalizes WP_VERSION and
VERSION_SUFFIX to also emit both values via GITHUB_OUTPUT, then change
REPORT_NAME in the artifact upload and summary-related expressions to read those
values through the step’s outputs instead of env.WP_VERSION and
env.VERSION_SUFFIX. Preserve the existing normalized values and PHP matrix
naming.
- Around line 7-9: Remove the workflow-wide actions: write entry from the
permissions block in the Playwright workflow, retaining contents: read. Do not
add replacement Actions permissions unless an existing step explicitly requires
the Actions API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b859826-d880-4a68-9eac-c879bc3bce74
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.eslintignore.eslintrc.js.github/workflows/playwright.yml.github/workflows/plugin-build.ymle2e/test-utils/interactions.tse2e/tests/block-editor.spec.ts
💤 Files with no reviewable changes (1)
- .eslintrc.js
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/tests/block-editor.spec.ts
- e2e/test-utils/interactions.ts
| permissions: | ||
| contents: read | ||
| actions: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow file around the relevant lines
cat -n .github/workflows/playwright.yml | sed -n '1,220p'
# Find artifact-related usage and permissions in the repository
rg -n "upload-artifact|download-artifact|artifact-metadata|permissions:" .github/workflows . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: gambitph/Interactions
Length of output: 6443
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n .github/workflows/playwright.yml | sed -n '1,220p'Repository: gambitph/Interactions
Length of output: 6167
🌐 Web query:
GitHub Actions upload-artifact permissions actions: write artifact-metadata write workflow permissions docs
💡 Result:
As of July 15, 2026, the artifact-metadata: write permission is a fine-grained GitHub Actions permission specifically required for interacting with artifact metadata APIs, such as creating artifact metadata storage records [1]. Key points regarding this permission: 1. Function: The artifact-metadata: write permission allows a workflow to create and manage artifact-related metadata [1]. It replaced the broader contents: write permission for these specific API operations as of early 2026 [1]. 2. Usage: It is primarily used by actions that generate or persist metadata for build artifacts, such as the actions/attest-build-provenance action when creating storage records [2][3][4]. 3. Requirements: If you are using an action that requires this capability (e.g., to create storage records for linked artifacts), you must explicitly define the permission in your workflow job configuration [2][3]: permissions: artifact-metadata: write # Other necessary permissions (e.g., id-token: write, attestations: write) 4. Context: Note that the standard actions/upload-artifact action does not require the artifact-metadata: write permission to upload artifacts themselves [5][6][7]. The artifact-metadata permission is specifically for advanced metadata operations accessed via the REST API [1][8]. If you encounter errors like Failed to create storage record, ensure the permission is correctly scoped within your workflow job permissions block [9].
Citations:
- 1: https://github.blog/changelog/2026-01-13-new-fine-grained-permission-for-artifact-metadata-is-now-generally-available/
- 2: https://docs.github.com/en/code-security/how-tos/secure-your-supply-chain/establish-provenance-and-integrity/upload-linked-artifacts
- 3: https://github.com/actions/attest-build-provenance/tree/63e64444a7857c20bbda4810126aabe30ef3b047
- 4: actions/attest-build-provenance@00014ed
- 5: https://github.com/actions/upload-artifact?tab=readme-ov-file
- 6: https://github.com/actions/upload-artifact/blob/v4/README.md
- 7: https://github.com/actions/upload-artifact/blob/main/README.md
- 8: https://docs.github.com/en/rest/orgs/artifact-metadata
- 9:
artifact-metadata: writeset, yetFailed to create storage recordactions/attest-build-provenance#781
Remove the workflow-wide actions: write permission .github/workflows/playwright.yml:7-9
actions/upload-artifact and actions/download-artifact don’t need it, so this can stay at contents: read unless another step actually needs the Actions API.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 9-9: overly broad permissions (excessive-permissions): actions: write is overly broad at the workflow level
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/playwright.yml around lines 7 - 9, Remove the
workflow-wide actions: write entry from the permissions block in the Playwright
workflow, retaining contents: read. Do not add replacement Actions permissions
unless an existing step explicitly requires the Actions API.
Source: Linters/SAST tools
| env: | ||
| SUMMARY_BODY: ${{ steps.mto.outputs.body }} | ||
| REPORT_NAME: playwright-report-php_${{ matrix.php_version }}-wp_${{ env.WP_VERSION }}-${{ env.VERSION_SUFFIX }} | ||
| ARTIFACT_ID: ${{ steps.artifact-upload-step.outputs.artifact-id }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/playwright.yml"
rg -n 'GITHUB_ENV|env\.(WP_VERSION|VERSION_SUFFIX)|name: playwright-report' "$file"
# Confirm artifact names include both matrix dimensions.
rg -n 'matrix\.(php_version|wp_version)' "$file"Repository: gambitph/Interactions
Length of output: 1030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/playwright.yml"
nl -ba "$file" | sed -n '60,150p'Repository: gambitph/Interactions
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/playwright.yml"
awk 'NR>=60 && NR<=150 { printf "%4d %s\n", NR, $0 }' "$file"Repository: gambitph/Interactions
Length of output: 4055
🌐 Web query:
GitHub Actions env context GITHUB_ENV values available in expressions upload-artifact with name step outputs
💡 Result:
In GitHub Actions, there are distinct mechanisms for managing data within a workflow, depending on whether you are using the env context, the GITHUB_ENV environment file, or step outputs [1][2][3]. The env context is used for variables defined in the workflow file (at the workflow, job, or step level) [4][5]. It is processed before the job is sent to the runner, meaning it is suitable for expressions in conditional statements [2][6]. GITHUB_ENV is a file on the runner used to persist environment variables for subsequent steps within the same job [5][3]. When you append a variable to this file (e.g., echo "VAR_NAME=value" >>
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/variables
- 2: https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-variables
- 3: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/workflow-commands.md
- 4: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 5: https://stackoverflow.com/questions/57968497/how-do-i-set-an-env-var-with-a-bash-expression-in-github-actions
- 6: https://docs.github.com/en/actions/concepts/workflows-and-actions/expressions
- 7: https://github.com/actions/upload-artifact?tab=readme-ov-file
- 8: https://github.com/actions/upload-artifact/blob/main/action.yml
- 9: https://raw.githubusercontent.com/actions/upload-artifact/main/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/playwright.yml"
awk 'NR>=1 && NR<=70 { printf "%4d %s\n", NR, $0 }' "$file"Repository: gambitph/Interactions
Length of output: 2600
Use step outputs for the artifact name. WP_VERSION and VERSION_SUFFIX are only written via $GITHUB_ENV, so ${{ env.WP_VERSION }} / ${{ env.VERSION_SUFFIX }} stay empty in the later actions/upload-artifact@v4 and summary expressions. That makes the matrix artifact names collide; emit the normalized values via $GITHUB_OUTPUT and read them as steps.<id>.outputs.* instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/playwright.yml around lines 139 - 142, Update the workflow
step that normalizes WP_VERSION and VERSION_SUFFIX to also emit both values via
GITHUB_OUTPUT, then change REPORT_NAME in the artifact upload and
summary-related expressions to read those values through the step’s outputs
instead of env.WP_VERSION and env.VERSION_SUFFIX. Preserve the existing
normalized values and PHP matrix naming.
Use .nvmrc as the single Node version source across CI workflows, reorder the Playwright test job to extract the build artifact before npm cache setup, and document the Node 22 requirement for e2e runs. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
build:e2escripts and a matrix sync tool driven byreadme.txttested-up-to versionsTest plan
npm run build:e2ecompletes successfullynpm run testpasses locally (3/3 block editor tests)Made with Cursor
Summary by CodeRabbit