Conversation
# Conflicts: # .github/workflows/pr-checks.yml # package-lock.json # package.json
There was a problem hiding this comment.
Pull request overview
This PR adds basic end-to-end (e2e) test infrastructure to the Create Block Theme plugin using Playwright and @wordpress/e2e-test-utils-playwright. The tests verify the two main plugin entry points: the Site Editor sidebar toolbar button and the Admin Landing Page functionality including theme creation workflows.
Changes:
- Added e2e test infrastructure with Playwright configuration
- Added two test suites covering the Site Editor sidebar and Admin Landing Page
- Added e2e job to the CI workflow to run tests automatically
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/landing-page.spec.js | Test suite for the admin landing page covering page load, export button, and blank theme creation workflow |
| specs/editor-sidebar.spec.js | Test suite for the Site Editor sidebar verifying the plugin toolbar button appears |
| playwright.config.js | Playwright configuration using WordPress scripts defaults with wp-env test port |
| package.json | Added @wordpress/e2e-test-utils-playwright dependency and npm scripts for running e2e tests |
| package-lock.json | Lock file update for new e2e testing dependency |
| .gitignore | Added artifacts/ directory to ignore Playwright test artifacts |
| .github/workflows/pr-checks.yml | Added e2e job to CI workflow that runs tests in wp-env and uploads artifacts on failure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jeryj
left a comment
jeryj
left a comment
There was a problem hiding this comment.
I tried testing this with a fresh install:
npm installnpm run buildnpm run test:e2e
And all the e2e tests passed. Then I tried running some of the tests in isolation and got a failure on "Create Block Theme — Admin Landing Page › create blank theme end-to-end". After the failure started, I couldn't get the test to pass anymore. I think it's a test cleanup thing, because it's failing due to the "Theme already exists" message.
Also, the timeout for the test is really long (100000ms) so it takes a minute and 40 seconds for the test to fail. I'm not sure what Gutenbergs is, but I feel sure it's less than that. I imagine 10 seconds is probably plenty enough for most actions?
Thanks for adding e2e tests to this! 🙌🏻
| execSync( | ||
| `npx wp-env run cli wp theme delete ${ E2E_THEME_SLUG }`, | ||
| { stdio: 'ignore' } |
There was a problem hiding this comment.
I haven't seen a need for this before. Could you explain why this is needed? Is there a precedence for this in Gutenberg already?
There was a problem hiding this comment.
There's no deleteTheme() method available in @wordpress/e2e-test-utils-playwright, and the REST API doesn't support theme deletion (/wp/v2/themes is read-only). Because we're testing this plugin creating a theme, I think WP-CLI is the best way to clean them up.
wp-env is already running at this point, so npx wp-env run cli should be OK to use here.
specs/landing-page.spec.js
Outdated
| } ) | ||
| .click(); | ||
|
|
||
| await page.waitForURL( /site-editor/ ); |
There was a problem hiding this comment.
Could we assert some kind of success? How do we know if it worked correctly besides going to the site-editor?
There was a problem hiding this comment.
Yes, makes sense! I've added a test for the theme being activated in f76d4d9.
| page.getByRole( 'button', { name: 'Create Block Theme' } ) | ||
| ).toBeVisible(); |
There was a problem hiding this comment.
Could we test that it works? What does it do when you click it?
There was a problem hiding this comment.
Yes, added a test for opening the sidebar in 4665e2e.
specs/landing-page.spec.js
Outdated
| await expect( | ||
| page.getByRole( 'button', { name: /Export/ } ) | ||
| ).toBeVisible(); |
There was a problem hiding this comment.
Can we assert that it works when clicked?
There was a problem hiding this comment.
Added a test to ensure that the export was downloaded successfully: 040ed22.
specs/landing-page.spec.js
Outdated
| await expect( page.getByLabel( /Theme name/i ) ).toBeVisible(); | ||
| } ); | ||
|
|
||
| test( 'create blank theme end-to-end', async ( { admin, page } ) => { |
There was a problem hiding this comment.
When I run this test file on its own, this "'create blank theme end-to-end'" test fails: npm run test:e2e specs/landing-page.spec.js
1) [chromium] › specs/landing-page.spec.js:70:2 › Create Block Theme — Admin Landing Page › create blank theme end-to-end
Test timeout of 100000ms exceeded.
Error: page.waitForURL: Target page, context or browser has been closed
=========================== logs ===========================
waiting for navigation until "load"
============================================================
87 | .click();
88 |
> 89 | await page.waitForURL( /site-editor/ );
| ^
90 | } );
91 | } );
92 |
There was a problem hiding this comment.
I think I've managed to fix this, and when I try npm run test:e2e specs/landing-page.spec.js locally it now passes. It seems like waiting for the full Site Editor to load will take a while, so I've changed it to just check if the navigation has been successful, which should be good enough for this test for now.
|
Thanks so much, @jeryj!
I think this should be fixed now, I'm able to run both test suites independently. Seems like we need to delete the test theme in the
We're just using the default timeout settings from |
jeryj
left a comment
jeryj
left a comment
There was a problem hiding this comment.
I think there's still something wrong with the cleanup of the theme deletion. I ran these tests:
- 🟢
npm run test:e2e specs/landing-page.spec.js - 🟢
npm run test:e2e - 🟢
npm run test:e2e specs/editor-sidebar.spec.js - 🔴
npm run test:e2e specs/landing-page.spec.js
The failure was the same as earlier - "theme already exists".
Oh - that's fine then. I looked a little more. I think it's because it's failing while waiting for the site redirection, which has a much longer default. If before the site-editor redirect check, you add a check for the success message, then it fails with a 5000ms timeout instead. |
| await page | ||
| .getByRole( 'button', { | ||
| name: 'Create and Activate Blank Theme', | ||
| } ) | ||
| .click(); |
There was a problem hiding this comment.
| await page | |
| .getByRole( 'button', { | |
| name: 'Create and Activate Blank Theme', | |
| } ) | |
| .click(); | |
| const createThemeButton = page.getByRole( 'button', { | |
| name: 'Create and Activate Blank Theme', | |
| } ); | |
| await createThemeButton.click(); | |
| await expect( createThemeButton ).toBeHidden(); | |
| await expect( | |
| page.getByText( | |
| 'Theme created successfully. The editor will now load.' | |
| ) | |
| ).toBeVisible(); |
There was a problem hiding this comment.
This makes the failure 5000ms instead of 100000ms
This adds basic e2e tests to this repo. The tests cover the two main plugin entry points:
Also adds an e2e job to the CI workflow that spins up wp-env, runs the e2e tests, and uploads the artifacts on failure. Adds test:e2e and test:e2e:ui npm scripts and gitignores the artifacts/ directory.