E2E: production-ready Playwright suite — unified config, sharded CI, new coverage, reliability fixes - #1899
Merged
arifulhoque7 merged 15 commits intoJul 27, 2026
Conversation
…-env local test plugins
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesThis PR consolidates Playwright execution into setup, E2E, and API projects; adds shard-aware reporting and authentication reuse; expands settings, dashboard, frontend-login, MailPoet, and REST API coverage; improves synchronization and environment handling; and documents the resulting coverage and remaining gaps. Playwright orchestration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
- Added methods to restore and save user sessions in BasicLoginPage. - Integrated session management into basicLogin and basicLoginAndPluginVisit methods. - Created authSession utility functions for handling session files. - Updated selectors to include new MailPoet settings. - Added MailPoetPage class for managing MailPoet module and registration forms. - Implemented tests for MailPoet registration and subscription validation. - Enhanced PostFormPage with methods for editing and deleting posts from the dashboard. - Introduced settings persistence verification tests in alphaSetupTest.spec.ts. - Created REST API tests for WPUF endpoints to ensure proper functionality. - Added wpEnvCli utility for executing WP-CLI commands in the test environment.
- Merged multiple Playwright configuration files into a single config to streamline test execution phases. - Updated test scripts to include new commands for setup, E2E, and API testing with improved logging and error handling. - Introduced new utility functions for handling optional fields and validating element presence without requiring visibility. - Enhanced registration and session management logic to handle scenarios where external services (e.g., Google Maps) may not be available. - Improved error handling in MailPoet registration tests to skip tests when prerequisites are not met. - Removed deprecated parallel configuration files and adjusted the summary generation script to accommodate new sharding logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
tests/e2e/pages/api/WpufApi.ts (1)
29-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBasic-auth username is hardcoded to
'admin'.
create()always buildsBasic admin:<password>, coupling this client tocreateAdminAppPassword()'s default user. Accept the username as a parameter so the client isn't silently wrong if used with a different role's app password later.♻️ Proposed refactor
- static async create(appPassword?: string): Promise<WpufApi> { + static async create(appPassword?: string, username = 'admin'): Promise<WpufApi> { const ctx = await pwRequest.newContext({ baseURL: Urls.baseUrl, ignoreHTTPSErrors: true }); const authHeader = appPassword - ? { Authorization: 'Basic ' + Buffer.from(`admin:${appPassword}`).toString('base64') } + ? { Authorization: 'Basic ' + Buffer.from(`${username}:${appPassword}`).toString('base64') } : {}; return new WpufApi(ctx, authHeader); }🤖 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 `@tests/e2e/pages/api/WpufApi.ts` around lines 29 - 35, Update WpufApi.create to accept a username parameter and use it when constructing the Basic Authorization header instead of hardcoding "admin"; preserve the existing appPassword-absent behavior and update its callers to pass the intended username.tests/e2e/utils/sharded-summary.js (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider pre-run cleanup to avoid stale shard results.
If a previous CI run crashes before
cleanupResultFiles()executes, leftoverparallel-results/shard-*-results.jsonfiles could be picked up and merged into the next run's summary bygetAllAvailableResultFiles(), skewing aggregate stats. Consider clearingparallel-results/at the start oftest:setup/test:allrather than relying solely on end-of-run cleanup.Also applies to: 153-169
🤖 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 `@tests/e2e/utils/sharded-summary.js` at line 45, Clear the parallel-results directory before test execution begins, such as in the test:setup/test:all startup flow, so stale shard-*-results.json files cannot be discovered by getAllAvailableResultFiles(). Reuse the existing cleanupResultFiles() helper or equivalent pre-run cleanup while preserving the current end-of-run cleanup behavior.tests/e2e/CLAUDE.md (1)
24-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
test:api/test:allin the quick-start.CI now runs
npm run test:all:ci(setup → shards → api), but this section only documentstest:setup/test:parallel/test:sharded, with no mention oftest:apiortest:all, even though both exist inpackage.jsonand the latter is what CI actually invokes.📝 Suggested addition
npm run test:setup # run setup suite first (alphaSetupTest) npm run test:parallel # run the 3 native shards sequentially npm run test:sharded # setup + shards in sequence +npm run test:api # run the API project only +npm run test:all # setup + shards + api in sequence (what CI runs)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@tests/e2e/CLAUDE.mdaround lines 24 - 29, Update the quick-start command
list in CLAUDE.md to document npm run test:api for the API suite and npm run
test:all for the complete setup → shards → API sequence. Also include their :ci
variants, especially test:all:ci, alongside the existing CI command
documentation.</details> <!-- cr-comment:v1:a07bf9c3a9b6d5c0986a3eb7 --> </blockquote></details> <details> <summary>tests/e2e/pages/postForm.ts (1)</summary><blockquote> `793-825`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Duplicates the math-captcha logic already in `createPostFE`.** The switch-case computing the captcha answer here mirrors the inline block in `createPostFE` (same file, ~lines 286-310). Extracting a shared helper avoids the two copies drifting (e.g., if a new operator symbol is added, only one place needs updating). <details> <summary>♻️ Proposed refactor: have createPostFE reuse the new helper</summary> ```diff - // Math Captcha - const operand1 = await this.page.textContent(Selectors.postForms.postFormsFrontendCreate.postMathCaptchaFormsFE.operand1); - const operand2 = await this.page.textContent(Selectors.postForms.postFormsFrontendCreate.postMathCaptchaFormsFE.operand2); - const operator = await this.page.textContent(Selectors.postForms.postFormsFrontendCreate.postMathCaptchaFormsFE.operator); - let result: number; - switch (operator) { - case '+': - result = Number(operand1) + Number(operand2); - break; - case '-': - result = Number(operand1) - Number(operand2); - break; - case 'X': - result = Number(operand1) * Number(operand2); - break; - case 'x': - result = Number(operand1) * Number(operand2); - break; - case '/': - result = Number(operand1) / Number(operand2); - break; - default: - throw new Error('Invalid operator'); - } - await this.validateAndFillStrings(Selectors.postForms.postFormsFrontendCreate.postMathCaptchaFormsFE.mathCaptcha, result.toString()); + // Math Captcha + await this.solveMathCaptchaIfPresent(); //Create Post await this.validateAndClick(Selectors.postForms.postFormsFrontendCreate.submitPostFormsFE);🤖 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 `@tests/e2e/pages/postForm.ts` around lines 793 - 825, Extract the shared operand/operator calculation from createPostFE into solveMathCaptchaIfPresent, and update createPostFE to invoke this helper instead of maintaining its own switch-case. Preserve the existing captcha detection, validation, and error behavior while ensuring the calculation logic has a single implementation.
🤖 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 `@tests/e2e/coverage-gap.md`:
- Line 405: Update the Playwright configuration reference in coverage-gap.md to
name tests/e2e/playwright.config.ts as the single configuration file, and
describe setup, e2e, and API phases as being selected through its projects
instead of referring to playwright.*.config.ts files.
- Around line 349-354: Update the captcha coverage entry in the coverage-gap
document to separate Math Captcha from reCaptcha, Turnstile, and Really Simple
captcha. Preserve the documented frontend post round-trip coverage for Math
Captcha, and list only the remaining captcha types as lacking enforced-submit
coverage.
- Line 368: Update the documented REST API spec reference in coverage-gap.md
from tests/api/wpufRestApi.spec.ts to tests/e2e/tests/api/wpufRestApi.spec.ts,
preserving the surrounding AP0001–AP0006 entry.
- Around line 47-48: Update the backlog text near the “Top of the backlog” entry
so the `#20` feature identifier is escaped at the start of its line, or keep it on
the preceding line; preserve the feature list while ensuring markdownlint does
not interpret it as a malformed heading.
- Line 129: Update the “Notification emails” gap description in coverage-gap.md
to hyphenate “post submission” as “post-submission,” leaving the rest of the
wording unchanged.
In `@tests/e2e/package.json`:
- Around line 16-23: Align the local E2E commands with the documented
headed-by-default behavior: in tests/e2e/package.json lines 16-23, add --headed
to test:setup and test:parallel so test:sharded inherits it; in
tests/e2e/CLAUDE.md lines 24-29, keep the documentation stating that CI variants
omit --headed and verify it matches the updated scripts.
In `@tests/e2e/pages/api/WpufApi.ts`:
- Around line 62-69: Update findSubscriptionIdByTitle to retrieve all paginated
/wpuf_subscription results before searching, continuing through response pages
until the matching title is found or no pages remain. Preserve support for both
body.subscriptions and body.result, and return the created subscription’s ID
when a match exists.
In `@tests/e2e/pages/base.ts`:
- Around line 319-345: Update the retry branch in waitForFormSaved so it tracks
whether the save-button retry and subsequent toast wait succeed. Log the
existing success message only when the retry completes successfully; when
retryError is caught, log a warning that the retry failed and include the error
details, while preserving the best-effort return-false behavior.
In `@tests/e2e/pages/postForm.ts`:
- Around line 835-849: Update deletePostFromDashboard so the dialog listener
registered before the dashboard menu interactions is always removed, including
when either validateAndClick call throws. Wrap the interaction and subsequent
deletion flow in try/finally, placing page.off('dialog', dialogHandler) in the
finally block while preserving the existing navigation, reload, and assertion
behavior.
In `@tests/e2e/pages/settingsSetup.ts`:
- Around line 832-856: Update validateGeneralSettingsPersistenceRoundTrip so the
sentinel write, save, reload, and toHaveValue verification are enclosed in a
try/finally block, with restoration of the captured original Google Maps API key
in finally. Keep the existing validation and reload behavior, but ensure
restoration runs even when the sentinel assertion or any preceding round-trip
step fails.
In `@tests/e2e/tests/mailpoetRegistrationTestPro.spec.ts`:
- Around line 22-26: Add a test.afterAll cleanup hook corresponding to the
beforeAll setup, closing the page/context as appropriate and terminating the
browser created by chromium.launch(). Keep the existing shared browser, context,
and page setup unchanged while ensuring all resources are released after the
suite completes.
In `@tests/e2e/utils/wpEnvCli.ts`:
- Around line 1-43: The wpCli and isMailPoetSubscriberInList helpers interpolate
untrusted values into shell commands and SQL. Replace execSync in wpCli with
execFileSync using an argument array that separates npx, wp-env, run, tests-cli,
wp, and command arguments; update createAdminAppPassword to pass user, label,
and flags as separate argv entries. Escape email and listName using the database
client’s supported SQL-escaping mechanism before composing the query in
isMailPoetSubscriberInList.
---
Nitpick comments:
In `@tests/e2e/CLAUDE.md`:
- Around line 24-29: Update the quick-start command list in CLAUDE.md to
document npm run test:api for the API suite and npm run test:all for the
complete setup → shards → API sequence. Also include their :ci variants,
especially test:all:ci, alongside the existing CI command documentation.
In `@tests/e2e/pages/api/WpufApi.ts`:
- Around line 29-35: Update WpufApi.create to accept a username parameter and
use it when constructing the Basic Authorization header instead of hardcoding
"admin"; preserve the existing appPassword-absent behavior and update its
callers to pass the intended username.
In `@tests/e2e/pages/postForm.ts`:
- Around line 793-825: Extract the shared operand/operator calculation from
createPostFE into solveMathCaptchaIfPresent, and update createPostFE to invoke
this helper instead of maintaining its own switch-case. Preserve the existing
captcha detection, validation, and error behavior while ensuring the calculation
logic has a single implementation.
In `@tests/e2e/utils/sharded-summary.js`:
- Line 45: Clear the parallel-results directory before test execution begins,
such as in the test:setup/test:all startup flow, so stale shard-*-results.json
files cannot be discovered by getAllAvailableResultFiles(). Reuse the existing
cleanupResultFiles() helper or equivalent pre-run cleanup while preserving the
current end-of-run cleanup 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac4856b1-5f6c-4314-89c7-75345a26fe9a
📒 Files selected for processing (30)
.github/workflows/e2e-wpuf.yml.gitignoreCLAUDE.mdtests/e2e/.gitignoretests/e2e/CLAUDE.mdtests/e2e/coverage-gap.mdtests/e2e/features-map/features-map.ymltests/e2e/package.jsontests/e2e/pages/api/WpufApi.tstests/e2e/pages/base.tstests/e2e/pages/basicLogin.tstests/e2e/pages/mailPoet.tstests/e2e/pages/postForm.tstests/e2e/pages/regForm.tstests/e2e/pages/selectors.tstests/e2e/pages/settingsSetup.tstests/e2e/pages/subscription.tstests/e2e/playwright.config.tstests/e2e/playwright.parallel-one.config.tstests/e2e/playwright.parallel-three.config.tstests/e2e/playwright.parallel-two.config.tstests/e2e/playwright.setup.config.tstests/e2e/tests/alphaSetupTest.spec.tstests/e2e/tests/api/wpufRestApi.spec.tstests/e2e/tests/mailpoetRegistrationTestPro.spec.tstests/e2e/tests/postFormTest.spec.tstests/e2e/tests/regFormTestPro.spec.tstests/e2e/utils/authSession.tstests/e2e/utils/sharded-summary.jstests/e2e/utils/wpEnvCli.ts
💤 Files with no reviewable changes (4)
- tests/e2e/playwright.setup.config.ts
- tests/e2e/playwright.parallel-two.config.ts
- tests/e2e/playwright.parallel-three.config.ts
- tests/e2e/playwright.parallel-one.config.ts
Comment on lines
+47
to
+48
| **Top of the backlog (build first):** #8 Payments (Stripe/PayPal txns), #9 Coupons & Tax math, | ||
| #20 REST API layer, #21 negative/security cases, #12 content restriction, #7 recurring/renewal |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape the feature ID at the start of line 48.
#20 is a feature identifier, not a heading; adding a space would change the document structure. Escape the hash or keep the sentence on line 47 so markdownlint passes without creating an unintended heading.
Based on static analysis, this heading is missing the required space after #.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 48-48: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 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 `@tests/e2e/coverage-gap.md` around lines 47 - 48, Update the backlog text near
the “Top of the backlog” entry so the `#20` feature identifier is escaped at the
start of its line, or keep it on the preceding line; preserve the feature list
while ensuring markdownlint does not interpret it as a malformed heading.
Source: Linters/SAST tools
| **Gaps:** | ||
| - 🔴 **Post expiration** (`../wpuf-pro/includes/Post_Expiration.php`) — expiry date, expired-post | ||
| status change, expiration email. Not tested. *(P1)* | ||
| - 🔴 **Notification emails** for post submission (admin/user) content not asserted via SMTP capture. *(P2)* |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hyphenate “post-submission”.
Use “post-submission” in the notification-email gap description for clearer grammar.
Based on static analysis, this wording requires a hyphen.
🧰 Tools
🪛 LanguageTool
[grammar] ~129-~129: Use a hyphen to join words.
Context: ...)* - 🔴 Notification emails for post submission (admin/user) content not asse...
(QB_NEW_EN_HYPHEN)
🤖 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 `@tests/e2e/coverage-gap.md` at line 129, Update the “Notification emails” gap
description in coverage-gap.md to hyphenate “post submission” as
“post-submission,” leaving the rest of the wording unchanged.
Source: Linters/SAST tools
|
|
||
| ## 20. REST API (`wpuf/v1`) — 🟡 (core layer built) — P1 for the rest | ||
|
|
||
| **API layer added (`AP0001`–`AP0006`, 2026-07-03):** `tests/api/wpufRestApi.spec.ts` + |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the REST API spec path.
The documented path tests/api/wpufRestApi.spec.ts does not match the actual path tests/e2e/tests/api/wpufRestApi.spec.ts. Update the reference so readers can locate the test.
Based on the supplied PR stack, the API spec lives under tests/e2e/tests/api/.
🤖 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 `@tests/e2e/coverage-gap.md` at line 368, Update the documented REST API spec
reference in coverage-gap.md from tests/api/wpufRestApi.spec.ts to
tests/e2e/tests/api/wpufRestApi.spec.ts, preserving the surrounding
AP0001–AP0006 entry.
Comment on lines
+319
to
+345
| // Detect the transient "Saved form data" toast with a generous timeout. | ||
| // IMPORTANT: always return false ("saved – stop") so callers that loop | ||
| // `while (flag) { create/build form; flag = waitForFormSaved(...) }` run | ||
| // exactly once. Returning true on a flaky false-negative made those loops | ||
| // re-enter and create DUPLICATE forms, which then broke unscoped | ||
| // form-name selectors with Playwright strict-mode violations. | ||
| try { | ||
| let formNotSaved = true; | ||
| let count = 1; | ||
| while (formNotSaved && count < 2) { | ||
| try { | ||
| await this.waitForLoading(); | ||
| await this.page.locator(formSavedLocator).waitFor({ timeout: 5000 }); | ||
| await this.waitForLoading(); | ||
| formNotSaved = false; | ||
| } catch (error) { | ||
| console.log('\x1b[33m%s\x1b[0m', `⚠️ Form not saved yet, clicking save button`); | ||
| await this.waitForLoading(); | ||
| await this.validateAndClick(saveButtonLocator); | ||
| await this.waitForLoading(); | ||
| count++; | ||
| } | ||
| } | ||
| await this.waitForLoading(); | ||
| await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 }); | ||
| await this.waitForLoading(); | ||
| console.log('\x1b[32m%s\x1b[0m', `✅ Form saved`); | ||
| return false; | ||
| } catch (error) { | ||
| console.log('\x1b[31m%s\x1b[0m', `❌ Failed to save form`); | ||
| return true; | ||
| // Toast not seen in time (slow env / already dismissed). Best-effort: | ||
| // click Save once more and wait again, but never propagate — treat the | ||
| // form as saved to avoid duplicate-form creation. | ||
| console.log('\x1b[33m%s\x1b[0m', `⚠️ Save toast not detected yet, clicking save once more`); | ||
| try { | ||
| await this.waitForLoading(); | ||
| await this.validateAndClick(saveButtonLocator); | ||
| await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 }); | ||
| } catch (retryError) { | ||
| // ignore – assume saved | ||
| } | ||
| await this.waitForLoading(); | ||
| console.log('\x1b[32m%s\x1b[0m', `✅ Form saved (after retry)`); | ||
| return false; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry branch always logs success even when the retry itself fails.
In the catch block, retryError is silently ignored and the code unconditionally logs "✅ Form saved (after retry)" and returns false, regardless of whether the retry's waitFor actually detected the toast. A genuine total-save-failure (button click fails or toast never appears even after retry) is now indistinguishable from a real save in the logs, which will make CI failures harder to diagnose later (the actual failure will surface downstream, disconnected from its root cause here).
🐛 Proposed fix to distinguish retry success from retry failure in logs
try {
await this.waitForLoading();
await this.validateAndClick(saveButtonLocator);
await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 });
+ console.log('\x1b[32m%s\x1b[0m', `✅ Form saved (after retry)`);
} catch (retryError) {
- // ignore – assume saved
+ console.log('\x1b[33m%s\x1b[0m', `⚠️ Retry save toast still not detected — proceeding anyway, this may indicate a real save failure`);
}
await this.waitForLoading();
- console.log('\x1b[32m%s\x1b[0m', `✅ Form saved (after retry)`);
return false;📝 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.
Suggested change
| // Detect the transient "Saved form data" toast with a generous timeout. | |
| // IMPORTANT: always return false ("saved – stop") so callers that loop | |
| // `while (flag) { create/build form; flag = waitForFormSaved(...) }` run | |
| // exactly once. Returning true on a flaky false-negative made those loops | |
| // re-enter and create DUPLICATE forms, which then broke unscoped | |
| // form-name selectors with Playwright strict-mode violations. | |
| try { | |
| let formNotSaved = true; | |
| let count = 1; | |
| while (formNotSaved && count < 2) { | |
| try { | |
| await this.waitForLoading(); | |
| await this.page.locator(formSavedLocator).waitFor({ timeout: 5000 }); | |
| await this.waitForLoading(); | |
| formNotSaved = false; | |
| } catch (error) { | |
| console.log('\x1b[33m%s\x1b[0m', `⚠️ Form not saved yet, clicking save button`); | |
| await this.waitForLoading(); | |
| await this.validateAndClick(saveButtonLocator); | |
| await this.waitForLoading(); | |
| count++; | |
| } | |
| } | |
| await this.waitForLoading(); | |
| await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 }); | |
| await this.waitForLoading(); | |
| console.log('\x1b[32m%s\x1b[0m', `✅ Form saved`); | |
| return false; | |
| } catch (error) { | |
| console.log('\x1b[31m%s\x1b[0m', `❌ Failed to save form`); | |
| return true; | |
| // Toast not seen in time (slow env / already dismissed). Best-effort: | |
| // click Save once more and wait again, but never propagate — treat the | |
| // form as saved to avoid duplicate-form creation. | |
| console.log('\x1b[33m%s\x1b[0m', `⚠️ Save toast not detected yet, clicking save once more`); | |
| try { | |
| await this.waitForLoading(); | |
| await this.validateAndClick(saveButtonLocator); | |
| await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 }); | |
| } catch (retryError) { | |
| // ignore – assume saved | |
| } | |
| await this.waitForLoading(); | |
| console.log('\x1b[32m%s\x1b[0m', `✅ Form saved (after retry)`); | |
| return false; | |
| // Detect the transient "Saved form data" toast with a generous timeout. | |
| // IMPORTANT: always return false ("saved – stop") so callers that loop | |
| // `while (flag) { create/build form; flag = waitForFormSaved(...) }` run | |
| // exactly once. Returning true on a flaky false-negative made those loops | |
| // re-enter and create DUPLICATE forms, which then broke unscoped | |
| // form-name selectors with Playwright strict-mode violations. | |
| try { | |
| await this.waitForLoading(); | |
| await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 }); | |
| await this.waitForLoading(); | |
| console.log('\x1b[32m%s\x1b[0m', `✅ Form saved`); | |
| return false; | |
| } catch (error) { | |
| // Toast not seen in time (slow env / already dismissed). Best-effort: | |
| // click Save once more and wait again, but never propagate — treat the | |
| // form as saved to avoid duplicate-form creation. | |
| console.log('\x1b[33m%s\x1b[0m', `⚠️ Save toast not detected yet, clicking save once more`); | |
| try { | |
| await this.waitForLoading(); | |
| await this.validateAndClick(saveButtonLocator); | |
| await this.page.locator(formSavedLocator).first().waitFor({ timeout: 15000 }); | |
| console.log('\x1b[32m%s\x1b[0m', `✅ Form saved (after retry)`); | |
| } catch (retryError) { | |
| console.log('\x1b[33m%s\x1b[0m', `⚠️ Retry save toast still not detected — proceeding anyway, this may indicate a real save failure`); | |
| } | |
| await this.waitForLoading(); | |
| return false; |
🤖 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 `@tests/e2e/pages/base.ts` around lines 319 - 345, Update the retry branch in
waitForFormSaved so it tracks whether the save-button retry and subsequent toast
wait succeed. Log the existing success message only when the retry completes
successfully; when retryError is caught, log a warning that the retry failed and
include the error details, while preserving the best-effort return-false
behavior.
Comment on lines
+835
to
+849
| // Delete a post from the dashboard, auto-accepting the "Are you sure to | ||
| // delete?" confirm dialog, and assert it is removed from the list. | ||
| async deletePostFromDashboard(title: string) { | ||
| await this.openPostsDashboard(); | ||
| const dialogHandler = async (dialog: Dialog) => { await dialog.accept(); }; | ||
| this.page.on('dialog', dialogHandler); | ||
| // Open the row's "⋮" options menu, then click Delete (fires the confirm). | ||
| await this.validateAndClick(Selectors.postForms.dashboardManage.optionsMenuTrigger(title)); | ||
| await this.validateAndClick(Selectors.postForms.dashboardManage.deleteLinkForPost(title)); | ||
| this.page.off('dialog', dialogHandler); | ||
| await this.page.waitForLoadState('domcontentloaded'); | ||
| await this.page.reload(); | ||
| await expect(this.page.locator(Selectors.postForms.dashboardManage.postTitleCell(title))).toHaveCount(0); | ||
| console.log('\x1b[32m%s\x1b[0m', `✅ Post deleted: "${title}" no longer in dashboard`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Dialog listener leaks if the click before Delete throws.
this.page.off('dialog', dialogHandler) only runs after both validateAndClick calls succeed. If either throws (e.g., options menu button not found), the listener stays registered on the page for the rest of the test, silently auto-accepting any later native dialog.
🔒️ Proposed fix: guarantee listener cleanup
const dialogHandler = async (dialog: Dialog) => { await dialog.accept(); };
this.page.on('dialog', dialogHandler);
- // Open the row's "⋮" options menu, then click Delete (fires the confirm).
- await this.validateAndClick(Selectors.postForms.dashboardManage.optionsMenuTrigger(title));
- await this.validateAndClick(Selectors.postForms.dashboardManage.deleteLinkForPost(title));
- this.page.off('dialog', dialogHandler);
+ try {
+ // Open the row's "⋮" options menu, then click Delete (fires the confirm).
+ await this.validateAndClick(Selectors.postForms.dashboardManage.optionsMenuTrigger(title));
+ await this.validateAndClick(Selectors.postForms.dashboardManage.deleteLinkForPost(title));
+ } finally {
+ this.page.off('dialog', dialogHandler);
+ }📝 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.
Suggested change
| // Delete a post from the dashboard, auto-accepting the "Are you sure to | |
| // delete?" confirm dialog, and assert it is removed from the list. | |
| async deletePostFromDashboard(title: string) { | |
| await this.openPostsDashboard(); | |
| const dialogHandler = async (dialog: Dialog) => { await dialog.accept(); }; | |
| this.page.on('dialog', dialogHandler); | |
| // Open the row's "⋮" options menu, then click Delete (fires the confirm). | |
| await this.validateAndClick(Selectors.postForms.dashboardManage.optionsMenuTrigger(title)); | |
| await this.validateAndClick(Selectors.postForms.dashboardManage.deleteLinkForPost(title)); | |
| this.page.off('dialog', dialogHandler); | |
| await this.page.waitForLoadState('domcontentloaded'); | |
| await this.page.reload(); | |
| await expect(this.page.locator(Selectors.postForms.dashboardManage.postTitleCell(title))).toHaveCount(0); | |
| console.log('\x1b[32m%s\x1b[0m', `✅ Post deleted: "${title}" no longer in dashboard`); | |
| } | |
| // Delete a post from the dashboard, auto-accepting the "Are you sure to | |
| // delete?" confirm dialog, and assert it is removed from the list. | |
| async deletePostFromDashboard(title: string) { | |
| await this.openPostsDashboard(); | |
| const dialogHandler = async (dialog: Dialog) => { await dialog.accept(); }; | |
| this.page.on('dialog', dialogHandler); | |
| try { | |
| // Open the row's "⋮" options menu, then click Delete (fires the confirm). | |
| await this.validateAndClick(Selectors.postForms.dashboardManage.optionsMenuTrigger(title)); | |
| await this.validateAndClick(Selectors.postForms.dashboardManage.deleteLinkForPost(title)); | |
| } finally { | |
| this.page.off('dialog', dialogHandler); | |
| } | |
| await this.page.waitForLoadState('domcontentloaded'); | |
| await this.page.reload(); | |
| await expect(this.page.locator(Selectors.postForms.dashboardManage.postTitleCell(title))).toHaveCount(0); | |
| console.log('\x1b[32m%s\x1b[0m', `✅ Post deleted: "${title}" no longer in dashboard`); | |
| } |
🤖 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 `@tests/e2e/pages/postForm.ts` around lines 835 - 849, Update
deletePostFromDashboard so the dialog listener registered before the dashboard
menu interactions is always removed, including when either validateAndClick call
throws. Wrap the interaction and subsequent deletion flow in try/finally,
placing page.off('dialog', dialogHandler) in the finally block while preserving
the existing navigation, reload, and assertion behavior.
Comment on lines
+832
to
+856
| async validateGeneralSettingsPersistenceRoundTrip() { | ||
| const sentinel = 'wpuf-qa-gmap-persist-check'; | ||
| await this.navigateToURL(this.wpufSettingsPage); | ||
| await this.page.reload(); | ||
| await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral); | ||
|
|
||
| // Turnstile enable toggle from setup persisted. | ||
| await expect(this.page.locator(Selectors.settingsSetup.persistence.turnstileEnableCheckbox)) | ||
| .toBeChecked(); | ||
|
|
||
| // Round-trip a text setting: capture original -> write sentinel -> save -> reload -> assert. | ||
| const gmapField = Selectors.settingsSetup.keys.fillGoogleMapAPIKey; | ||
| const original = await this.page.locator(gmapField).inputValue(); | ||
| await this.page.locator(gmapField).fill(sentinel); | ||
| await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave); | ||
| await this.page.reload(); | ||
| await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral); | ||
| await expect(this.page.locator(gmapField)).toHaveValue(sentinel); | ||
| console.log('\x1b[32m%s\x1b[0m', '✅ WPUF general setting round-trip persisted (Google Map API key)'); | ||
|
|
||
| // Restore the original value so downstream tests see the pre-existing state. | ||
| await this.page.locator(gmapField).fill(original); | ||
| await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave); | ||
| await this.page.reload(); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restore-on-failure gap in the round-trip persistence check.
If the assertion at Line 849 (toHaveValue(sentinel)) fails, the restore block at Lines 853-855 never runs, leaving the Google Map API key permanently set to 'wpuf-qa-gmap-persist-check' server-side. Any later test/run depending on the real key (e.g. optional Google Map field flows) would then silently break.
Wrap the write/verify in try/finally so the original value is always restored:
🛡️ Proposed fix
const gmapField = Selectors.settingsSetup.keys.fillGoogleMapAPIKey;
const original = await this.page.locator(gmapField).inputValue();
- await this.page.locator(gmapField).fill(sentinel);
- await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave);
- await this.page.reload();
- await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral);
- await expect(this.page.locator(gmapField)).toHaveValue(sentinel);
- console.log('\x1b[32m%s\x1b[0m', '✅ WPUF general setting round-trip persisted (Google Map API key)');
-
- // Restore the original value so downstream tests see the pre-existing state.
- await this.page.locator(gmapField).fill(original);
- await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave);
- await this.page.reload();
+ try {
+ await this.page.locator(gmapField).fill(sentinel);
+ await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave);
+ await this.page.reload();
+ await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral);
+ await expect(this.page.locator(gmapField)).toHaveValue(sentinel);
+ console.log('\x1b[32m%s\x1b[0m', '✅ WPUF general setting round-trip persisted (Google Map API key)');
+ } finally {
+ // Restore the original value so downstream tests see the pre-existing state,
+ // even if the assertion above failed.
+ await this.page.locator(gmapField).fill(original);
+ await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave);
+ await this.page.reload();
+ }📝 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.
Suggested change
| async validateGeneralSettingsPersistenceRoundTrip() { | |
| const sentinel = 'wpuf-qa-gmap-persist-check'; | |
| await this.navigateToURL(this.wpufSettingsPage); | |
| await this.page.reload(); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral); | |
| // Turnstile enable toggle from setup persisted. | |
| await expect(this.page.locator(Selectors.settingsSetup.persistence.turnstileEnableCheckbox)) | |
| .toBeChecked(); | |
| // Round-trip a text setting: capture original -> write sentinel -> save -> reload -> assert. | |
| const gmapField = Selectors.settingsSetup.keys.fillGoogleMapAPIKey; | |
| const original = await this.page.locator(gmapField).inputValue(); | |
| await this.page.locator(gmapField).fill(sentinel); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave); | |
| await this.page.reload(); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral); | |
| await expect(this.page.locator(gmapField)).toHaveValue(sentinel); | |
| console.log('\x1b[32m%s\x1b[0m', '✅ WPUF general setting round-trip persisted (Google Map API key)'); | |
| // Restore the original value so downstream tests see the pre-existing state. | |
| await this.page.locator(gmapField).fill(original); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave); | |
| await this.page.reload(); | |
| } | |
| async validateGeneralSettingsPersistenceRoundTrip() { | |
| const sentinel = 'wpuf-qa-gmap-persist-check'; | |
| await this.navigateToURL(this.wpufSettingsPage); | |
| await this.page.reload(); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral); | |
| // Turnstile enable toggle from setup persisted. | |
| await expect(this.page.locator(Selectors.settingsSetup.persistence.turnstileEnableCheckbox)) | |
| .toBeChecked(); | |
| // Round-trip a text setting: capture original -> write sentinel -> save -> reload -> assert. | |
| const gmapField = Selectors.settingsSetup.keys.fillGoogleMapAPIKey; | |
| const original = await this.page.locator(gmapField).inputValue(); | |
| try { | |
| await this.page.locator(gmapField).fill(sentinel); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave); | |
| await this.page.reload(); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.clickSettingsTabGeneral); | |
| await expect(this.page.locator(gmapField)).toHaveValue(sentinel); | |
| console.log('\x1b[32m%s\x1b[0m', '✅ WPUF general setting round-trip persisted (Google Map API key)'); | |
| } finally { | |
| // Restore the original value so downstream tests see the pre-existing state, | |
| // even if the assertion above failed. | |
| await this.page.locator(gmapField).fill(original); | |
| await this.validateAndClick(Selectors.settingsSetup.keys.settingsTabGeneralSave); | |
| await this.page.reload(); | |
| } | |
| } |
🤖 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 `@tests/e2e/pages/settingsSetup.ts` around lines 832 - 856, Update
validateGeneralSettingsPersistenceRoundTrip so the sentinel write, save, reload,
and toHaveValue verification are enclosed in a try/finally block, with
restoration of the captured original Google Maps API key in finally. Keep the
existing validation and reload behavior, but ensure restoration runs even when
the sentinel assertion or any preceding round-trip step fails.
Comment on lines
+22
to
+26
| test.beforeAll(async () => { | ||
| browser = await chromium.launch(); | ||
| context = await browser.newContext(); | ||
| page = await context.newPage(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Browser/context launched but never closed.
chromium.launch() and the resulting context/page are created in beforeAll but there's no test.afterAll to close them. Under the required workers: 1 / sequential-shard execution for this suite, this leaks a browser process for the rest of that worker's run.
🔧 Proposed fix
test.beforeAll(async () => {
browser = await chromium.launch();
context = await browser.newContext();
page = await context.newPage();
});
+
+test.afterAll(async () => {
+ await context?.close();
+ await browser?.close();
+});📝 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.
Suggested change
| test.beforeAll(async () => { | |
| browser = await chromium.launch(); | |
| context = await browser.newContext(); | |
| page = await context.newPage(); | |
| }); | |
| test.beforeAll(async () => { | |
| browser = await chromium.launch(); | |
| context = await browser.newContext(); | |
| page = await context.newPage(); | |
| }); | |
| test.afterAll(async () => { | |
| await context?.close(); | |
| await browser?.close(); | |
| }); |
🤖 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 `@tests/e2e/tests/mailpoetRegistrationTestPro.spec.ts` around lines 22 - 26,
Add a test.afterAll cleanup hook corresponding to the beforeAll setup, closing
the page/context as appropriate and terminating the browser created by
chromium.launch(). Keep the existing shared browser, context, and page setup
unchanged while ensuring all resources are released after the suite completes.
Source: Coding guidelines
Comment on lines
+1
to
+43
| import { execSync } from 'child_process'; | ||
|
|
||
| /** | ||
| * Run a wp-cli command inside the wp-env "tests-cli" container and return stdout. | ||
| * | ||
| * Works both locally and in CI because both bring the environment up with | ||
| * `wp-env` (see .github/workflows/e2e-wpuf.yml). `--skip-plugins --skip-themes` | ||
| * keeps the call fast and avoids the dokan-lite CLI fatal. Use this only for | ||
| * verifying side effects a plugin writes to the DB that have no stable UI | ||
| * surface (e.g. MailPoet subscribers, whose admin UI is gated by onboarding). | ||
| */ | ||
| export function wpCli(command: string): string { | ||
| const full = `npx wp-env run tests-cli wp --skip-plugins --skip-themes ${command}`; | ||
| return execSync(full, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); | ||
| } | ||
|
|
||
| /** | ||
| * Create a fresh WordPress Application Password for the given admin user and | ||
| * return it (spaces stripped) for use as HTTP Basic auth against the REST API. | ||
| * | ||
| * Works locally and in CI because both bring the env up with wp-env. Application | ||
| * Passwords are retrievable only at creation time, so a new one is minted per run; | ||
| * this is harmless in the disposable test env. Used by the REST API test layer to | ||
| * authenticate `wpuf/v1` requests without coupling them to a browser login. | ||
| */ | ||
| export function createAdminAppPassword(user = 'admin', label = 'wpuf-e2e-api'): string { | ||
| const raw = wpCli(`user application-password create ${user} ${label} --porcelain`); | ||
| return raw.replace(/\s+/g, ''); | ||
| } | ||
|
|
||
| /** | ||
| * Return true if the given email is a MailPoet subscriber inside the named list | ||
| * (segment). Used to assert WPUF's "subscribe on registration" behavior. | ||
| */ | ||
| export function isMailPoetSubscriberInList(email: string, listName: string): boolean { | ||
| const sql = | ||
| 'SELECT s.email FROM wp_mailpoet_subscribers s ' + | ||
| 'JOIN wp_mailpoet_subscriber_segment ss ON ss.subscriber_id = s.id ' + | ||
| 'JOIN wp_mailpoet_segments seg ON seg.id = ss.segment_id ' + | ||
| `WHERE s.email = '${email}' AND seg.name = '${listName}';`; | ||
| const out = wpCli(`db query "${sql}"`); | ||
| return out.includes(email); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does wp-env runpass its command array to the container without invoking an additional shell, and doeswp db query support parameterized/escaped values instead of raw SQL string interpolation?
💡 Result:
The wp-env run command passes its arguments directly to the container without invoking an additional shell, effectively executing the command verbatim after your local shell has performed its own initial processing [1]. This behavior was intentionally updated to ensure that arguments—including those involving environment variables or complex quoting—are handled as expected by the container environment [1]. Regarding wp db query, it does not support parameterized or escaped values through an internal binding mechanism [2][3]. The command is designed to execute arbitrary SQL strings directly against the database using the credentials defined in wp-config.php [2][3]. Because it lacks native support for prepared statements or parameterized input, you must manually sanitize and escape any user-supplied data before passing it to the command to prevent SQL injection [4][5]. For programmatic needs requiring secure SQL execution within WordPress, the $wpdb->prepare() method remains the standard, authoritative approach for handling parameterized queries [4][6][5].
Citations:
- 1: Reworked
runCommand Parsing WordPress/gutenberg#50559 - 2: https://wpcli.dev/docs/db/query
- 3: https://packagist.org/packages/wp-cli/db-command
- 4: https://tweakswp.com/wordpress-sql-injection-prevention-wpdb-prepare-deep-dive/
- 5: https://developer.wordpress.org/reference/classes/wpdb/?output_format=md
- 6: https://github.com/WordPress/WordPress/blob/4a6f90db58a935abb688cfb91b391dffeda7b35c/wp-includes/wp-db.php
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline tests/e2e/utils/wpEnvCli.ts --view expanded || true
printf '\n== File contents ==\n'
cat -n tests/e2e/utils/wpEnvCli.ts
printf '\n== Usages of helpers ==\n'
rg -n "wpCli\\(|createAdminAppPassword\\(|isMailPoetSubscriberInList\\(" tests/e2e -S || trueRepository: weDevsOfficial/wp-user-frontend
Length of output: 3716
Avoid shell and SQL interpolation here
execSync() still goes through a shell, and wp db query only accepts raw SQL, so passing command, user, label, email, or listName through string interpolation is brittle and becomes shell/SQL injection as soon as any non-literal input reaches this helper. Use execFileSync()/argv for wpCli() and escape the SQL values before composing the query.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execSync } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 OpenGrep (1.25.0)
[ERROR] 14-14: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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 `@tests/e2e/utils/wpEnvCli.ts` around lines 1 - 43, The wpCli and
isMailPoetSubscriberInList helpers interpolate untrusted values into shell
commands and SQL. Replace execSync in wpCli with execFileSync using an argument
array that separates npx, wp-env, run, tests-cli, wp, and command arguments;
update createAdminAppPassword to pass user, label, and flags as separate argv
entries. Escape email and listName using the database client’s supported
SQL-escaping mechanism before composing the query in isMailPoetSubscriberInList.
Source: Linters/SAST tools
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/e2e/pages/api/WpufApi.ts (1)
62-79: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSubscription lookup still only fetches page 1; a second helper now repeats the same gap.
GET /wpuf_subscriptiondefaults toper_page=10, so once more than 10 packs exist (easy to hit as CRUD/edit/XSS tests accumulate across CI runs, or if a prior cleanup fails),findSubscriptionIdByTitleand the newly addedfindSubscriptionByTitleContainscan silently miss the target record. This causesvalidateSubscriptionCrudRoundTrip(Lines 137-162),validateSubscriptionEditRoundTrip(Lines 243-265), andvalidateXssTitleSanitized(Lines 301-324) to intermittently fail/false-negative. The pagination gap onfindSubscriptionIdByTitlewas flagged previously;findSubscriptionByTitleContainsduplicates the same unpaginated pattern in this PR.🐛 Proposed fix: paginate through all results once, share the fetch
- private async findSubscriptionIdByTitle(title: string): Promise<number | null> { - const res = await this.get('/wpuf_subscription'); - expect(res.status()).toBe(200); - const body = await res.json(); - const items = (body.subscriptions || body.result || []) as Array<Record<string, unknown>>; - const match = items.find( ( s ) => String( s.post_title ) === title ); - return match ? Number( match.ID ?? match.id ) : null; - } - - // Find the first subscription whose title *contains* the token; returns the - // matched record (with its stored title) so callers can assert on it. - private async findSubscriptionByTitleContains(token: string): Promise<Record<string, unknown> | null> { - const res = await this.get('/wpuf_subscription'); - expect(res.status()).toBe(200); - const body = await res.json(); - const items = (body.subscriptions || body.result || []) as Array<Record<string, unknown>>; - return items.find( ( s ) => String( s.post_title ).includes( token ) ) ?? null; - } + private async fetchAllSubscriptions(): Promise<Array<Record<string, unknown>>> { + const all: Array<Record<string, unknown>> = []; + let page = 1; + for (;;) { + const res = await this.get(`/wpuf_subscription?per_page=100&page=${page}`); + expect(res.status()).toBe(200); + const body = await res.json(); + const items = (body.subscriptions || body.result || []) as Array<Record<string, unknown>>; + all.push(...items); + if (items.length < 100) break; + page += 1; + } + return all; + } + + private async findSubscriptionIdByTitle(title: string): Promise<number | null> { + const items = await this.fetchAllSubscriptions(); + const match = items.find( ( s ) => String( s.post_title ) === title ); + return match ? Number( match.ID ?? match.id ) : null; + } + + // Find the first subscription whose title *contains* the token; returns the + // matched record (with its stored title) so callers can assert on it. + private async findSubscriptionByTitleContains(token: string): Promise<Record<string, unknown> | null> { + const items = await this.fetchAllSubscriptions(); + return items.find( ( s ) => String( s.post_title ).includes( token ) ) ?? null; + }🤖 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 `@tests/e2e/pages/api/WpufApi.ts` around lines 62 - 79, Update findSubscriptionIdByTitle and findSubscriptionByTitleContains to search across all /wpuf_subscription pages instead of only the default first page. Introduce a shared helper for fetching and aggregating paginated subscription results, then have both lookup methods reuse it while preserving their exact-match and contains-match 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.
Duplicate comments:
In `@tests/e2e/pages/api/WpufApi.ts`:
- Around line 62-79: Update findSubscriptionIdByTitle and
findSubscriptionByTitleContains to search across all /wpuf_subscription pages
instead of only the default first page. Introduce a shared helper for fetching
and aggregating paginated subscription results, then have both lookup methods
reuse it while preserving their exact-match and contains-match behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d6485d7-af2a-4f7d-b178-0a0bf8f24ab2
📒 Files selected for processing (9)
tests/e2e/coverage-gap.mdtests/e2e/features-map/features-map.ymltests/e2e/pages/api/WpufApi.tstests/e2e/pages/postForm.tstests/e2e/pages/regForm.tstests/e2e/pages/selectors.tstests/e2e/tests/api/wpufRestApi.spec.tstests/e2e/tests/postFormTest.spec.tstests/e2e/utils/wpEnvCli.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/pages/selectors.ts
- tests/e2e/pages/regForm.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/e2e-wpuf.yml (3)
201-220: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winPreserve the matrix shard selection.
The matrix still defines feature-specific
files, butnpm run test:all:ciignores them, causing every shard to execute the full suite and duplicate results. Restore the shard-specific command or maketest:all:ciforward${{ matrix.files }}while retaining the project selection inplaywright.config.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 @.github/workflows/e2e-wpuf.yml around lines 201 - 220, Update the “Run e2e tests (${{ matrix.group }})” step so it uses the matrix.files shard selection instead of always invoking the full-suite test:all:ci command. Restore the shard-specific test command or pass matrix.files through test:all:ci, while preserving the existing Playwright project selection from playwright.config.ts.Source: Learnings
38-40: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't exclude the repos this workflow is meant to run in. In
.github/workflows/e2e-wpuf.yml:38-40,232-235, bothe2eandmerge-reportsare gated bygithub.repository != 'weDevsOfficial/wp-user-frontend' && github.repository != 'weDevsOfficial/wpuf-pro', so they skip in the target repo instead of running there. Remove this guard or narrow it to the intended skip case.🤖 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/e2e-wpuf.yml around lines 38 - 40, Update the e2e and merge-reports jobs in the workflow so they are not excluded when running in the intended wp-user-frontend or wpuf-pro repositories. Remove the current github.repository inequality guard or narrow it only to the actual repositories that should be skipped, preserving all other job conditions.
14-22: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable persisted credentials on the build-job checkouts
pro_repo/pro_refcan swap in arbitrarywpuf-procode, and the following Composer/npm/Grunt steps run in the same workspace. Setpersist-credentials: falseon the checkout steps in this job, especially thewpuf-procheckout, so credentials aren’t written to disk before untrusted code runs.🤖 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/e2e-wpuf.yml around lines 14 - 22, Update the checkout steps in the build job, including the wpuf-pro checkout that uses pro_repo and pro_ref, to set persist-credentials to false. Apply this to every checkout step in that job so credentials are not written to the workspace before Composer, npm, or Grunt runs.Source: Linters/SAST tools
🤖 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 @.gitignore:
- Around line 80-83: Remove the broad /assets/* rule from .gitignore and retain
the existing individual generated-asset output patterns, ensuring hand-written
and third-party assets such as assets/vendor remain trackable.
---
Outside diff comments:
In @.github/workflows/e2e-wpuf.yml:
- Around line 201-220: Update the “Run e2e tests (${{ matrix.group }})” step so
it uses the matrix.files shard selection instead of always invoking the
full-suite test:all:ci command. Restore the shard-specific test command or pass
matrix.files through test:all:ci, while preserving the existing Playwright
project selection from playwright.config.ts.
- Around line 38-40: Update the e2e and merge-reports jobs in the workflow so
they are not excluded when running in the intended wp-user-frontend or wpuf-pro
repositories. Remove the current github.repository inequality guard or narrow it
only to the actual repositories that should be skipped, preserving all other job
conditions.
- Around line 14-22: Update the checkout steps in the build job, including the
wpuf-pro checkout that uses pro_repo and pro_ref, to set persist-credentials to
false. Apply this to every checkout step in that job so credentials are not
written to the workspace before Composer, npm, or Grunt runs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0c39787-b0fa-41a8-9e3e-8e5eb096a07a
📒 Files selected for processing (4)
.github/workflows/e2e-wpuf.yml.gitignoretests/e2e/pages/selectors.tstests/e2e/playwright.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/pages/selectors.ts
- tests/e2e/playwright.config.ts
Comment on lines
+80
to
+83
|
|
||
| # wp-env local test plugins scaffold (mounted by tests/e2e/.wp-env.json) | ||
| /plugins/ | ||
| /assets/* No newline at end of file |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not ignore the entire assets directory.
The file already lists the generated asset outputs individually, while /assets/* also hides hand-written and third-party files—and overrides the existing !assets/vendor exception. Remove this broad rule and retain only the generated-output patterns.
Proposed fix
# wp-env local test plugins scaffold (mounted by tests/e2e/.wp-env.json)
/plugins/
-/assets/*📝 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.
Suggested change
| # wp-env local test plugins scaffold (mounted by tests/e2e/.wp-env.json) | |
| /plugins/ | |
| /assets/* | |
| # wp-env local test plugins scaffold (mounted by tests/e2e/.wp-env.json) | |
| /plugins/ |
🤖 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 @.gitignore around lines 80 - 83, Remove the broad /assets/* rule from
.gitignore and retain the existing individual generated-asset output patterns,
ensuring hand-written and third-party assets such as assets/vendor remain
trackable.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-wpuf.yml (1)
226-226: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winDo not run the complete suite in every matrix job.
matrix.filesis now unused. Each of the three jobs runs setup, all three E2E shards, and API tests viatest:all:ci, tripling CI work rather than partitioning it. Remove this matrix or make each job invoke only its assigned native shard.As per coding guidelines, shard the
e2eproject with Playwright’s native--shard=i/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 @.github/workflows/e2e-wpuf.yml at line 226, Update the matrix jobs around test:all:ci so they no longer run the complete suite redundantly. Remove the unused matrix.files configuration or change each job to run only its assigned e2e shard using Playwright’s native --shard=i/n option, while preserving the intended three-way partitioning and keeping API tests outside the per-shard E2E commands.Source: Coding guidelines
🤖 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 `@tests/e2e/playwright.config.ts`:
- Line 68: Update the Playwright configuration’s workers setting from 4 to 1 so
stateful specs run sequentially against the shared WordPress site, preserving
the suite’s single-worker behavior.
- Around line 72-75: Update the Playwright reporter configuration to restore the
blob reporter alongside the existing list and JSON reporters, writing artifacts
to the tests/e2e/blob-report location expected by CI and playwright
merge-reports.
In `@tests/e2e/tests/frontendLoginTest.spec.ts`:
- Around line 35-45: Update the setup and teardown around seedPageWithShortcode
and seedUser to retain the created page and user IDs, then delete both resources
in afterAll after restoring the WPUF options. Ensure cleanup targets only the
page and flspecuser created by this spec so shared-site state is preserved
across reruns.
In `@tests/e2e/utils/siteReady.ts`:
- Around line 12-26: Update waitForSiteReady to throw an error when the deadline
is reached without a successful probe. Before each request and polling delay,
calculate the remaining budget and cap both the request timeout and
waitForTimeout duration to that remaining time, while preserving immediate
return on res.ok().
---
Outside diff comments:
In @.github/workflows/e2e-wpuf.yml:
- Line 226: Update the matrix jobs around test:all:ci so they no longer run the
complete suite redundantly. Remove the unused matrix.files configuration or
change each job to run only its assigned e2e shard using Playwright’s native
--shard=i/n option, while preserving the intended three-way partitioning and
keeping API tests outside the per-shard E2E commands.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b1db3aa-e1ac-4735-8517-96d6cb2af347
📒 Files selected for processing (15)
.github/workflows/e2e-wpuf.ymlGruntfile.jstests/e2e/coverage-gap.mdtests/e2e/features-map/features-map.ymltests/e2e/package.jsontests/e2e/pages/frontendLogin.tstests/e2e/pages/selectors.tstests/e2e/pages/settingsSetup.tstests/e2e/playwright.config.tstests/e2e/tests/frontendLoginTest.spec.tstests/e2e/tests/postFormTest.spec.tstests/e2e/tests/regFormSettingsTestPro.spec.tstests/e2e/tests/regFormTestPro.spec.tstests/e2e/utils/siteReady.tstests/e2e/utils/wpEnvCli.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/e2e/pages/settingsSetup.ts
- tests/e2e/tests/regFormTestPro.spec.ts
- tests/e2e/pages/selectors.ts
- tests/e2e/tests/postFormTest.spec.ts
- tests/e2e/package.json
Comment on lines
+35
to
+45
| // Seed via wp-cli (fast, self-cleaning): page with [wpuf-login], registered | ||
| // as WPUF's login page so the form posts back to itself. | ||
| const seeded = seedPageWithShortcode(loginPageTitle, '[wpuf-login]'); | ||
| loginUrl = seeded.url; | ||
| loginPageBefore = getWpufOptionKey('wpuf_profile', 'login_page'); | ||
| setWpufOptionKey('wpuf_profile', 'login_page', String(seeded.id)); | ||
|
|
||
| turnstileBefore = getWpufOptionKey('wpuf_general', 'enable_turnstile'); | ||
| setWpufOptionKey('wpuf_general', 'enable_turnstile', 'off'); | ||
|
|
||
| seedUser(feUser, feUserEmail, feUserPassword); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Clean up the seeded page and user.
afterAll restores options only; the shortcode page and flspecuser created here remain on the shared site after each run. Capture their IDs and delete them during teardown so reruns do not alter later specs’ shared state.
Also applies to: 48-61
🤖 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 `@tests/e2e/tests/frontendLoginTest.spec.ts` around lines 35 - 45, Update the
setup and teardown around seedPageWithShortcode and seedUser to retain the
created page and user IDs, then delete both resources in afterAll after
restoring the WPUF options. Ensure cleanup targets only the page and flspecuser
created by this spec so shared-site state is preserved across reruns.
Comment on lines
+12
to
+26
| export async function waitForSiteReady(page: Page, maxMs: number = 30000): Promise<void> { | ||
| const probeUrl = `${Urls.baseUrl}/wp-login.php`; | ||
| const deadline = Date.now() + maxMs; | ||
| while (Date.now() < deadline) { | ||
| try { | ||
| const res = await page.request.get(probeUrl, { timeout: 10000 }); | ||
| if (res.ok()) { | ||
| return; | ||
| } | ||
| } catch { | ||
| // site not up yet — keep polling | ||
| } | ||
| await page.waitForTimeout(1000); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'siteReady\.ts$' . || true
echo "== inspect target file =="
if [ -f tests/e2e/utils/siteReady.ts ]; then
cat -n tests/e2e/utils/siteReady.ts
fi
echo "== usages =="
rg -n "waitForSiteReady|siteReady" tests/e2e -S || true
echo "== package scripts for JS linting =="
if [ -f package.json ]; then
jq '.scripts // {}' package.json
fiRepository: weDevsOfficial/wp-user-frontend
Length of output: 3322
Fail when readiness times out and bound each probe to the remaining budget.
Exiting the loop after Date.now() >= deadline lets callers continue against a site that is still unavailable, and the page.request.get(..., { timeout: 10000 }) / waitForTimeout(1000) can overshoot the configured readiness ceiling.
Proposed fix
- while (Date.now() < deadline) {
+ while (true) {
+ const remainingMs = deadline - Date.now();
+ if (remainingMs <= 0) {
+ throw new Error(`Site did not become ready within ${maxMs}ms`);
+ }
try {
- const res = await page.request.get(probeUrl, { timeout: 10000 });
+ const res = await page.request.get(probeUrl, {
+ timeout: Math.min(10000, remainingMs),
+ });
if (res.ok()) {
return;
}
} catch {
// site not up yet — keep polling
}
- await page.waitForTimeout(1000);
+ await page.waitForTimeout(Math.min(1000, deadline - Date.now()));
}📝 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.
Suggested change
| export async function waitForSiteReady(page: Page, maxMs: number = 30000): Promise<void> { | |
| const probeUrl = `${Urls.baseUrl}/wp-login.php`; | |
| const deadline = Date.now() + maxMs; | |
| while (Date.now() < deadline) { | |
| try { | |
| const res = await page.request.get(probeUrl, { timeout: 10000 }); | |
| if (res.ok()) { | |
| return; | |
| } | |
| } catch { | |
| // site not up yet — keep polling | |
| } | |
| await page.waitForTimeout(1000); | |
| } | |
| } | |
| export async function waitForSiteReady(page: Page, maxMs: number = 30000): Promise<void> { | |
| const probeUrl = `${Urls.baseUrl}/wp-login.php`; | |
| const deadline = Date.now() + maxMs; | |
| while (true) { | |
| const remainingMs = deadline - Date.now(); | |
| if (remainingMs <= 0) { | |
| throw new Error(`Site did not become ready within ${maxMs}ms`); | |
| } | |
| try { | |
| const res = await page.request.get(probeUrl, { | |
| timeout: Math.min(10000, remainingMs), | |
| }); | |
| if (res.ok()) { | |
| return; | |
| } | |
| } catch { | |
| // site not up yet — keep polling | |
| } | |
| await page.waitForTimeout(Math.min(1000, deadline - Date.now())); | |
| } | |
| } |
🤖 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 `@tests/e2e/utils/siteReady.ts` around lines 12 - 26, Update waitForSiteReady
to throw an error when the deadline is reached without a successful probe.
Before each request and polling delay, calculate the remaining budget and cap
both the request timeout and waitForTimeout duration to that remaining time,
while preserving immediate return on res.ok().
…registration flow
Registration CI shard failed on two 180s timeouts: - EM0003 (MailPoet subscribe-on-registration) hung because the MailPoet plugin was never installed, so no list existed and the save-retry loop span forever. Install MailPoet in .wp-env.json + the CI "Install Plugins" step (default "Newsletter mailing list" now present), and bound the save-retry loop (max 5 attempts) so a stuck save fails loudly instead of spinning to the test timeout. - RF0014 (WC Vendor email verification) hung clicking a Register button that never enabled (env-dependent WC Vendors frontend validation). Mirror the RF0009 self-skip pattern: return null when Register stays disabled and cascade-skip RF0014-RF0017 instead of hanging. Pipeline hardening: - Raise WP_MAX_MEMORY_LIMIT to 512M in .wp-env.json — MailPoet activation alongside WooCommerce exhausts the default 128M and fatals. - Add concurrency (cancel-in-progress) so overlapping runs never mutate the single stateful wp-env at once. - Add least-privilege permissions (contents: read). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9iDQG1jPJ63uKvBFrwEX1
The prior flow clicked Register twice. WC Vendors re-disables the button while the first submit is in flight, so the second click waited on a disabled button until the 180s test timeout (CI: "locator.click: Target page closed"). Click the enabled locator exactly once, then wait for the success message within a bounded window; if it never appears, return null so RF0014-RF0017 self-skip instead of hanging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9iDQG1jPJ63uKvBFrwEX1
All three e2e shards pass, but merge-reports failed because the Send-Test-Report email steps error with "530 Authentication Required" on a fork without SMTP_EMAIL_* secrets. Email is a notification, not a gate — add continue-on-error so SMTP availability never decides build status. Test outcome is still enforced by "Reflect shard results in run status". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9iDQG1jPJ63uKvBFrwEX1
The prior concurrency group keyed on github.ref, so every manual dispatch on the same branch cancelled the previous one (and re-runs from the UI cancelled in-flight dispatches mid "Build WPUF-pro"). Key the group on the PR number when present, else github.run_id (unique per run), and only cancel-in-progress for pull_request events. Superseded PR pushes still cancel; manual, branch and scheduled runs now run to completion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9iDQG1jPJ63uKvBFrwEX1
dev-shahed
changed the title
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
2 participants
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.
What this PR does
Overhauls the WPUF Playwright E2E suite into a production-ready pipeline: one unified config, native sharding, new feature coverage (frontend login, REST API, MailPoet subscribe-on-registration, settings persistence), faster session-reused logins, and a green CI run with the flaky/environment-dependent paths made self-skipping instead of hanging.
Scope is test tooling + CI only — no plugin runtime code changes.
Highlights
Test infrastructure
playwright.config.tswith selectableprojects—setup/e2e/api— replacing the old per-phase configs (playwright.setup/parallel-one/two/three.config.ts).e2eis split with Playwright's native--shard=i/n.utils/authSession.ts+basicLogin.ts): first login per role cachesstorageStateto.auth/<role>.json; later specs re-inject cookies (self-healing if stale). Big wall-clock win across the stateful suite.wpuf-test-automationskill (.claude/skills/…) documenting how to author/run/extend the suite.utils/siteReady.ts) and a wp-env CLI helper (utils/wpEnvCli.ts) for DB-level assertions.New coverage
tests/api/wpufRestApi.spec.ts+pages/api/WpufApi.ts) — the browserlesswpuf/v1project: status codes, schema,permission_callbackauthz.frontendLoginTest.spec.ts,pages/frontendLogin.ts).mailpoetRegistrationTestPro.spec.ts,pages/mailPoet.ts) — email-marketing module coverage; MailPoet plugin wired into.wp-env.json+ CI.features-map.yml, and a livingcoverage-gap.md.Reliability — hang → self-skip
Environment-dependent paths now detect the missing prerequisite and skip cleanly instead of clicking a dead element and burning the 180s test timeout:
CI / pipeline hardening (
.github/workflows/e2e-wpuf.yml)post,registration,fields-subscription), blob-report merge → HTML + Summary table, artifacts uploaded.concurrency: PR runs auto-cancel a superseded push; manual/branch/schedule runs get a unique group so they never cancel each other.permissions: contents: read(least privilege).continue-on-error) — SMTP availability never decides build status; the tests do, via Reflect shard results.WP_MAX_MEMORY_LIMIT=512Min wp-env — MailPoet + WooCommerce activation exceeds the default 128M.How to run
Test evidence
Full suite green on the fork CI (all three e2e shards + merge-reports):
dev-shahed/wp-user-frontendrun 30248142343 —post,registration,fields-subscription,merge-reportsall ✅.Notes
workers: 1,fullyParallel: false,retries: 0(a retry would re-run an ordered test against mutated shared state). Real cross-runner parallelism would need a wp-env per matrix job.tests/e2e/**, the workflow, the skill, and.gitignore/CLAUDE.mddocs.🤖 Generated with Claude Code