fix(e2e): stabilize Cypress login on custom login pages#1282
Conversation
MERGE_SUMMARYSummary
Verification
Commit: 1827b0d |
📝 WalkthroughWalkthroughThe Cypress login command is made more resilient to varying page structures. The ChangesLogin form robustness improvements
Possibly related PRs
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/cypress/support/commands/login.js`:
- Around line 45-49: The indentation in the added block using
cy.get("body").then(...) uses spaces; update the indentation to use tabs
consistent with the project's JS coding guidelines. Locate the block containing
cy.get("body").then(($body) => { ... }) and replace the leading spaces on each
line inside that function (including the if ($body.find("`#rememberme`")...) line
and the cy.get("`#rememberme`")... line) with tabs so the file uses tabs for
indentation throughout.
- Line 59: Update the assertion that currently reads `.and("include",
"/wp-admin")` to use an anchored regex match so the pathname must start with
`/wp-admin` (e.g., change to a `.and("match", /^\/wp-admin/)` style check) and
convert the indentation on that line (and its surrounding lines if affected)
from spaces to a single tab to comply with the project JS indentation rule; look
for the assertion chain involving `.and("include", "/wp-admin")` in the login
command and replace it with the anchored regex match and tab-indented line.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8836d2f-1d5f-4a48-885c-f29abd1c72d0
📒 Files selected for processing (1)
tests/e2e/cypress/support/commands/login.js
| cy.get("body").then(($body) => { | ||
| if ($body.find("#rememberme").length) { | ||
| cy.get("#rememberme").should("be.visible").and("not.be.checked").click(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Use tabs for indentation per coding guidelines.
The added lines use spaces for indentation, but the coding guidelines require tabs for all *.js files. As per coding guidelines, "indentation must be tabs".
🛠️ Proposed fix to convert spaces to tabs
- cy.get("body").then(($body) => {
- if ($body.find("`#rememberme`").length) {
- cy.get("`#rememberme`").should("be.visible").and("not.be.checked").click();
- }
- });
+ cy.get("body").then(($body) => {
+ if ($body.find("`#rememberme`").length) {
+ cy.get("`#rememberme`").should("be.visible").and("not.be.checked").click();
+ }
+ });📝 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.
| cy.get("body").then(($body) => { | |
| if ($body.find("#rememberme").length) { | |
| cy.get("#rememberme").should("be.visible").and("not.be.checked").click(); | |
| } | |
| }); | |
| cy.get("body").then(($body) => { | |
| if ($body.find("`#rememberme`").length) { | |
| cy.get("`#rememberme`").should("be.visible").and("not.be.checked").click(); | |
| } | |
| }); |
🤖 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/cypress/support/commands/login.js` around lines 45 - 49, The
indentation in the added block using cy.get("body").then(...) uses spaces;
update the indentation to use tabs consistent with the project's JS coding
guidelines. Locate the block containing cy.get("body").then(($body) => { ... })
and replace the leading spaces on each line inside that function (including the
if ($body.find("`#rememberme`")...) line and the cy.get("`#rememberme`")... line)
with tabs so the file uses tabs for indentation throughout.
| .should("not.contain", "/wp-login.php") | ||
| .and("not.contain", "/login") | ||
| .and("equal", "/wp-admin/"); | ||
| .and("include", "/wp-admin"); |
There was a problem hiding this comment.
Use tabs for indentation and tighten the pathname assertion.
Two concerns:
- The line uses spaces for indentation, but the coding guidelines require tabs for all
*.jsfiles. - Using
.include("/wp-admin")is overly permissive and would match unintended paths like/some-path/wp-adminor/not-wp-admin. To ensure the pathname starts with/wp-admin, use an anchored regex instead.
As per coding guidelines, "indentation must be tabs".
🛠️ Proposed fix
- .and("include", "/wp-admin");
+ .and("match", /^\/wp-admin/);📝 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.
| .and("include", "/wp-admin"); | |
| .and("match", /^\/wp-admin/); |
🤖 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/cypress/support/commands/login.js` at line 59, Update the assertion
that currently reads `.and("include", "/wp-admin")` to use an anchored regex
match so the pathname must start with `/wp-admin` (e.g., change to a
`.and("match", /^\/wp-admin/)` style check) and convert the indentation on that
line (and its surrounding lines if affected) from spaces to a single tab to
comply with the project JS indentation rule; look for the assertion chain
involving `.and("include", "/wp-admin")` in the login command and replace it
with the anchored regex match and tab-indented line.
SummaryFollow-up for #1281 after the original PR merged while Cypress was still running.
Verification
Merged via PR #1282 to main. aidevops.sh v3.19.5 spent 37s on this as a headless bash routine. |
|
Performance Test Results Performance test results for 98d7902 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
superdav42
added
the
review-feedback-scanned
1 participant
Summary
Follow-up for #1281 after the original PR merged while Cypress was still running.
cy.loginByForm()tolerate custom login pages that omit#rememberme./wp-adminpath, including network admin.main.Verification
node --check tests/e2e/cypress/support/commands/login.js— passed.php -l tests/e2e/cypress/fixtures/setup-password-reset-subsite.php— passed.vendor/bin/phpcs tests/e2e/cypress/fixtures/setup-password-reset-subsite.php— passed.wp eval-file tests/e2e/cypress/fixtures/setup-password-reset-subsite.php— returnedfilter_ran: true, non-wp-login.phprewritten URL, and preservedwp_lang.npx cypress run -b chrome --config-file cypress.config.test.js --spec 'tests/e2e/cypress/integration/011-password-reset-subsite-domain.spec.js,tests/e2e/cypress/integration/030-modal-form-error-handling.spec.js'— local environment blocked becausehttp://localhost:8889was not running; CI is authoritative for the wp-env Cypress target.For #1281
Summary by CodeRabbit