add front end tests (WP-1011)#628
Conversation
Adds Playwright infrastructure to catch the class of bug where backend nonce checks are added without updating legacy PHP-rendered JavaScript. Core test (ajax-security.spec.js) intercepts every admin-ajax.php POST on the post edit and bulk submit pages, asserts _wpnonce is present in each request, and asserts no 403 responses. This test would have caught the WP-1007 regression in ContentEditJob.php. CI integration: Node.js 20 + Playwright Chromium baked into the Docker image; test.sh starts a PHP built-in server before PHPUnit and exits non-zero if either E2E or PHP tests fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Dockerfile: install @playwright/test globally (npm install -g) so test.sh has no dependency on package-lock.json at runtime - test.sh: drop the npm ci step; use 'playwright' binary directly (available globally in the Docker image) - Commit package-lock.json for reproducible local npm install Fixes: npm ci failing (no lockfile), then npx downloading wrong playwright version (1.61.1 instead of @playwright/test@1.47.2). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-1011)
require('@playwright/test') in playwright.config.js fails when running
from the project dir because Node's resolution never reaches the global
prefix. NODE_PATH=$(npm root -g) makes the global install visible.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r (WP-1011)
wp server uses the CWD as document root when --docroot is not specified.
At the point the server is started, CWD is ${PLUGIN_DIR}/inc/third-party/bin,
so WordPress files ended up accessible at http://test.com/WP_INSTALL_DIR/
instead of http://test.com/. This caused wp_login_url() to generate
redirect URLs with /WP_INSTALL_DIR in the path, breaking the auth setup.
Also reset siteurl/home to http://${E2E_DOMAIN} before starting the server
as a defensive measure in case multisite-convert left a stale path in the
options table.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wp server --docroot is unreliable across wp-cli versions — the previous commit showed the server stopped responding entirely when --docroot was added. Switch to php -S 0.0.0.0:80 -t WP_INSTALL_DIR which is explicit and unambiguous: document root is always WP_INSTALL_DIR. Our test URLs (wp-login.php, wp-admin/post.php, wp-admin/admin.php) are direct PHP files so WordPress rewrite routing is not needed, making the wp server router script unnecessary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PavelLoparev
left a comment
PavelLoparev
left a comment
There was a problem hiding this comment.
Review
Overview: Adds a Playwright E2E layer to catch regressions like WP-1007 (legacy jQuery AJAX call missing _wpnonce). Scope: CI infra (Dockerfile-Jenkins, test.sh E2E section: multisite domain normalization, custom static-file router, mu-plugin blocking outbound HTTP during tests, OPcache pre-warming, fixture creation), the Playwright suite itself (auth.setup.js, ajax-security.spec.js, job-wizard.spec.js), a WP-CLI fixture script, and a small js/app.js fix (swallow network errors in loadRelations instead of an unhandled rejection).
Correctness / Test Coverage
-
Bulk-submit-page nonce test can pass vacuously. In
ajax-security.spec.js, the post-edit-page test assertssmartlingCalls.length > 0before checking nonces; the bulk-submit-page version drops that assertion ("Bulk submit page may not trigger AJAX on load — only assert when calls exist"). Butjs/app.js(JobWizard'suseEffect(() => { loadJobs(); }, [loadJobs])) fires unconditionally regardless ofisBulkSubmitPage— same#smartling-appcontainer, same mount path. So if the bulk-submit page's AJAX call broke entirely (exactly the class of regression this PR targets),smartlingCallsandmissingwould both be[], andtoHaveLength(0)would pass with zero real coverage. Suggest mirroring the post-edit test'sexpect(smartlingCalls.length).toBeGreaterThan(0)guard here. -
Only half of WP-1007's DoD is covered. WP-1007's DoD was "All ajax endpoints both verify nonces and check if user is capable of performing action." This suite verifies nonce presence / no-403s while authenticated as full admin, but doesn't test the capability-check side (no lower-privilege user scenario). Worth a follow-up ticket or a note in the PR description so reviewers don't assume that half is covered.
Code Quality
- The new E2E section in
test.shis heavily and usefully commented given how much multisite/WordPress-bootstrap subtlety it's navigating — good for future maintainers. - Nice cleanup: removed leftover
DIAGdebug echoes from the namespacer section. js/app.js's newcatch (e) {}is silent (comment-only), inconsistent with the siblingloadJobscatch which callssetError(...). Probably fine since relations-loading is best-effort, but aconsole.warnwould help future debugging.- Minor:
Dockerfile-Jenkinspipes NodeSource's setup script throughbashunpinned — standard for CI images but worth knowing as an unpinned supply-chain point on every rebuild.
Performance
- 900s timeout ceiling, OPcache pre-warming, static-file router bypassing WP bootstrap, and blocking outbound HTTP via mu-plugin are all sensible, well-justified mitigations for the CI failure modes described (clearly the product of the preceding "fix Playwright ..." commits).
Security
- No production security issues. The HTTP-blocking/router/FLUpdater shims are CI-only and don't ship in the release zip (only
${PLUGIN_DIR}is packaged).
Suggested changes before merge
- Add the missing
expect(smartlingCalls.length).toBeGreaterThan(0)guard to the bulk-submit nonce test (or document why it's intentionally different). - Consider noting the WP-1007 capability-check gap in the PR description.
Nice addition overall — approving, with the above as non-blocking follow-ups.
2 participants
No description provided.