Add - Themegrill QA Setup for E2E Testing - #1662
Merged
Merged
Conversation
…iew-on-comment Add PR review automation
composer.json declares '>=5.6.20' (an ancient, unmaintained floor), but the real composer.lock needs 8.2+ (phpspec/prophecy, webmozart/assert both require it). Confirmed for real: composer install failed outright on the previous 7.4 default.
@ajna/pagination@1.4.19 peer-requires react-scripts@5.0.1 -> old TypeScript, but this project pins typescript@5.9.3 -- a genuine conflict confirmed via npm ERESOLVE, not a build-config issue. --legacy-peer-deps is a pragmatic workaround scoped to this repo only (via the install-command override), not a change to the reusable workflow or any other repo. Does not fix the underlying conflict -- that still needs this repo's own team (drop/replace @ajna/pagination, or reconcile the TypeScript version).
npm run build runs this repos own fully self-contained release script (its own npm install, composer install, JS build, and rsync+zip). Our external composer-install and custom rsync+zip wrapper were redundant work duplicating it, and never the actual point of failure -- the real failing composer call lives inside the script itself, fixed separately in a PR to bin/build-zip.sh.
composer.lock has dev-only packages (phpunit and friends) locked to versions requiring old PHP, while the production dependencies need a newer PHP -- no single installed PHP version satisfies both at once. Composer validates the whole lock file's platform requirements up front regardless of which packages get installed, so this script's build failed outright in CI on any modern PHP version. --ignore-platform-reqs skips that validation. Safe here: this script never actually runs the dev-only tools it temporarily installs (no phpunit execution happens before the --no-dev cleanup pass strips them back out), it only needs them present long enough to satisfy whatever step expects vendor/ to exist.
Verified working: install-command needs npm ci --legacy-peer-deps (not a no-op -- cross-env must be resolvable before npm run build can even start), and install-command values must never contain embedded double quotes (breaks the reusable workflow's own shell test, confirmed for real). composer-install: false and build-command: npm run build are correct as-is, matching bin/build-zip.sh's own self-contained packaging.
…omposer-platform-reqs fix: bin/build-zip.sh composer install --ignore-platform-reqs
Add PR build ZIP workflow
…iew-on-comment Add PR review automation
QA suite — passed ✅All 11 checks passed. 11 passed · 0 failed · 0 skipped · 1 flaky · 318s
|
| Test | Spec | Retries |
|---|---|---|
| a colour preset chosen in the panel reaches the rendered form @fresh @style-customizer | tests/e2e/specs/style-customizer/color-preset-reaches-front-end.spec.ts:49 |
1 |
See it for yourself
Download the full report (qa-suite-everest-forms-1662.zip). Unzip it and open qa-report.html in any browser — it shows each failure with screenshots of the page at the moment it broke, and what the run did and did not check.
Replaying a failure step by step (developers)
The archive also carries a Playwright trace — every click, the page at each step, network and console. From the unzipped folder:
npx playwright show-report playwright-report
It needs that command rather than opening the file directly: a trace viewer cannot start from a file:// page.
Automated check — no AI involved. It runs the tests in this branch.
There was a problem hiding this comment.
🟡 Changes recommended
It introduces CI/runtime incompatibilities (Playwright requires Node >=20 while existing CI still uses Node 12) and includes a couple of correctness/consistency issues (test cleanup URL param handling, lockfile version mismatch).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a Playwright-based end-to-end (E2E) test suite for Everest Forms, plus ThemeGrill QA suite metadata and CI wiring, to add browser-level regression coverage for critical user flows and key regressions between 3.4.8–3.6.0 without changing production plugin code.
Changes:
- Adds a new Playwright E2E suite (
tests/e2e/) with fixtures, auth setup, and 19@freshspecs covering core flows and targeted regressions. - Adds ThemeGrill QA suite configuration and ingested docs/knowledge index under
.themegrill-qa/. - Updates CI/workflows and build scripts to support running the suite and producing testable ZIPs; adds
@playwright/testanddotenvdevDependencies.
File summaries
| File | Description |
|---|---|
| tests/e2e/tsconfig.json | TypeScript config for the E2E suite. |
| tests/e2e/specs/submission/entry-end-to-end.spec.ts | Critical “visitor submission persists to Entries” end-to-end coverage. |
| tests/e2e/specs/style-customizer/style-customizer-v2.spec.ts | Guards Style Customizer V2 panel mounting/structure. |
| tests/e2e/specs/style-customizer/color-preset-reaches-front-end.spec.ts | Verifies a saved preset affects frontend computed styles. |
| tests/e2e/specs/settings/validation-message-persists.spec.ts | Asserts settings persistence across save + reload. |
| tests/e2e/specs/settings/captcha-language-persists.spec.ts | Regression guard for CAPTCHA language persistence. |
| tests/e2e/specs/forms/create-form-from-template.spec.ts | Template picker + builder-open regression coverage. |
| tests/e2e/specs/builder/pro-gated-controls.spec.ts | Guards free-vs-pro gated builder controls behavior. |
| tests/e2e/specs/builder/field-option-groups.spec.ts | Regression guard for option group toggle single-click behavior. |
| tests/e2e/specs/admin/menus-and-pages.spec.ts | Admin menu/analytics/system-info surface regression checks. |
| tests/e2e/specs/activation/admin-screens-render-clean.spec.ts | Ensures admin pages and frontend render without PHP errors. |
| tests/e2e/README.md | Suite documentation, conventions, and gaps. |
| tests/e2e/playwright.config.ts | Playwright runner config (serial execution, reporters, projects). |
| tests/e2e/fixtures/published-form.ts | Fixture to publish a real page hosting a form for anonymous visitor tests. |
| tests/e2e/fixtures/form.ts | Shared per-spec-file form fixture created via template picker. |
| tests/e2e/env.ts | Env resolution/precedence and dotenv loading for suite configuration. |
| tests/e2e/auth.setup.ts | Setup project that logs in once and persists storage state. |
| tests/e2e/.gitignore | Ignores E2E auth state and env artifacts under tests/e2e. |
| package.json | Adds Playwright test script and devDependencies (@playwright/test, dotenv). |
| package-lock.json | Locks Playwright/dotenv dependencies (and updates lock metadata). |
| bin/build-zip.sh | Adjusts Composer install flags for build ZIP script. |
| .themegrill-qa/suite.json | ThemeGrill QA suite manifest (tiers, areas, runner command). |
| .themegrill-qa/knowledge.md | Draft QA knowledge base used to justify/organize regression guards. |
| .themegrill-qa/docs/uncategorised.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/troubleshooting.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/translation.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/tools.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/page-builder-compatibility.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/integrations.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/global-settings.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/getting-started.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/getting-started-with-form.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/developers-doc.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/design.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/anti-spam-and-security.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs/account-management.md | Ingested docs section content (reference/intent source). |
| .themegrill-qa/docs-index.json | Index of ingested documentation sections and outcomes. |
| .themegrill-qa/.docs-cache/_wp-json_wp_v2_doc_category_per_page_100__fields_id_name_slug_count_parent.txt | Cached docs ingest API response. |
| .themegrill-qa/.docs-cache/_wp-json_wp_v2_docs_per_page_100_page_1__fields_id_slug_link_title_content_doc_category.txt | Cached docs ingest API response. |
| .themegrill-qa/.docs-cache/_wp-json_wp_v2_docs_per_page_100_page_2__fields_id_slug_link_title_content_doc_category.txt | Cached docs ingest API response. |
| .gitignore | Ignores local QA env and Playwright artifacts at repo root. |
| .github/workflows/qa-suite.yml | Adds CI workflow to run QA suite via ThemeGrill reusable workflow. |
| .github/workflows/pr-build-zip.yml | Updates ZIP build workflow wiring/notes. |
| .github/workflows/copilot-review-on-comment.yml | Adjusts reusable workflow source and adds PR-open trigger. |
| .distignore | Excludes Playwright reports/results from build artifacts. |
Review details
- Files reviewed: 51/56 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…sing Adds a @fresh guard for the 3.6.1 fix "Query parameters in external redirect URLs were HTML-encoded, causing incorrect values" (e265d5e). The spec asserts on the *second* query parameter, which is the minimum meaningful case: esc_url() encoded "&" as "&", the "#" then opened a fragment, and every parameter after the first fell out of the query silently. A single-parameter redirect passes with the bug present. Proved both ways — with esc_url_raw() restored to esc_url() the spec fails on a landed URL of "?evfqa=<stamp>#38;second=kept", and passes on the fix. Also reworks auth.setup.ts to report why login failed rather than guess. It now distinguishes a rejected login (reads #login_error), a PHP fatal in admin bootstrap, and an unexplained missing admin bar (reports landed URL and body prefix). The previous message offered "wrong credentials, or the site is not reachable" for all three, which is unactionable on CI — plugins load on wp-login.php but admin-only hooks do not, so an admin-side fatal presents exactly like a bad password. Documents four things that each cost a debugging cycle: networkidle is not a save signal in the builder, smart tags are not processed in the confirmation success message, the external-URL setting strips braces on save, and the Country smart-tag fix is consequently not reachable from any free surface. Suite: 20 @fresh tests, all passing against 3.6.1.
The diagnostic added in 8e687fe read `#login_error` with `innerText()`, which auto-waits for the element to exist. When there is no login error — the successful path and the PHP-fatal path both — that burned the full actionability timeout (~30s) before the catch swallowed it, leaving too little of the test budget for the assertion it was added to serve. CI reported "Test timeout of 45000ms exceeded" in authenticate, naming where the test stopped rather than the cause. It never appeared locally because storedStateStillWorks() finds a cached .auth/admin.json and returns before the login block. CI has no cache, so only CI ever took the path. Reproduced locally by deleting tests/e2e/.auth: cold authenticate went 1.0m and failing against a healthy site, and 1.2s passing with count() guarding the read. Audited every other innerText/textContent/getAttribute call in the suite; all are guarded by a count() or a preceding assertion. Documents the trap, and that a local run must clear tests/e2e/.auth to exercise what CI exercises. Suite: 20 @fresh tests, all passing from a cold start.
Node/engines: @playwright/test 1.63 declares `engines.node >= 20`, but package.json still claimed `>=8.9.3` and evf-PR-test.yml installed Node 12. Declare the real minimum (node >=20, npm >=8) and bump that workflow to Node 20.x, matching pr-build-zip.yml. Note npm 6 only WARNs on an engine mismatch, so this was not failing installs -- engines was simply inaccurate and CI ran a Node the project no longer supports. Playwright is not pinned down instead, since that would mean dropping the suite, and engine-strict is deliberately not set so contributors on older Node get a warning rather than a hard failure. Template spec cleanup: the URL assertion accepts (form_id|id) but cleanup read only form_id, so a redirect carrying `id=` skipped teardown and leaked the created form. Read both, and fix the failure message which still named only form_id. auth.setup.ts: give `context` an explicit BrowserContext | undefined instead of relying on inference. Not a type error -- evolving-any infers it from the assignment and tsc 5.9 passes under --strict --noImplicitAny -- but the explicit type is clearer and matches the inline import() style used above. package-lock.json: version said 3.6.0 while package.json said 3.6.1; the 3.6.1 bump never regenerated the lock. No package versions change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
Adds a Playwright E2E suite. The plugin had no browser-level tests, so nothing
guarded the flows a release actually breaks.
19
@freshtests, all passing locally:and the entry is verified on the Entries screen, values included. This is the
flow the product exists for and had no coverage at all.
each traced to its changelog entry in the spec's
@source: the StyleCustomizer revamp (3.6.0), the CAPTCHA language reset, the repeater control
Pro gate, the field-settings double-click, the Payments rename and System
Info addons list (3.5.3), plus the conditional-logic upsell and free
Analytics export/print (3.5.0).
error, settings persist across a reload, forms can be created from a template.
Also adds the
.themegrill-qa/QA config (suite manifest witharea_paths,ingested docs index, knowledge file) and a
qa-suite.ymlCI workflow.No product code is touched. The only changes to existing files are
package.json/package-lock.json(adds@playwright/testanddotenvasdevDependencies),
.gitignore, and.distignore.How to test the changes in this Pull Request:
npm install --legacy-peer-deps && npx playwright install chromium(
--legacy-peer-depsis required — this repo has a real peer conflict, seea33051d66)..themegrill-qa/.env.localwithTGQA_BASE_URL,TGQA_ADMIN_USER,TGQA_ADMIN_PASSpointing at a site with Everest Forms active.npm run test:e2e -- --grep @fresh— expect 19 passed.Types of changes:
Other information: