check failing tests#2460
Conversation
WalkthroughTest updates across three Cypress suites: selector changes and spacing in FormTemplates.cy.js; expanded accepted H1 headings and minor hook/cleanup edits in formPageDataValidation.cy.js; and revised submit-click scoping and wait logic in formsSettings.cy.js. No exported/public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Cypress test
participant Page as App page
Test->>Page: wait for .frm_forms to exist (timeout)
Test->>Page: locate submit element (button or input) within .frm_forms
Test->>Page: ensure submit is visible
Test->>Page: click submit
Page-->>Test: navigate/redirect (assert URL)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (6)
tests/cypress/e2e/Form Templates/FormTemplates.cy.js (6)
90-95: Remove duplicate “Edit User Profile Template” assertions to avoid redundancy/flakes.This block is a duplicate of the assertions on Lines 84–89 and adds runtime without additional value. Duplicate checks can cause confusion and increase flake risk if the DOM changes between the two blocks.
Apply this diff to remove the duplicate:
- cy.log("Edit User Profile Template"); - cy.get('[frm-search-text="edit user profile"] .frm-form-templates-item-title-text') - .should('contain', 'Edit User Profile'); - cy.get('[frm-search-text="edit user profile"] .frm-form-templates-item-description') - .should('contain.text', 'The basics from the regular WordPress profile page including first and last name, password and email, avatar, website, and bio.');
314-319: Avoid double-clicking “Use Template”; likely cause of modal/state conflicts.Clicking the same “Use Template” button twice back-to-back can open two overlays or race the UI state, leading to intermittent failures.
Apply this diff to remove the duplicate click:
- cy.get('li[frm-search-text="contact us"]').first() - .trigger('mouseover', { force: true }) - .find('.frm-form-templates-use-template-button') - .should('contain', 'Use Template') - .click({ force: true });
371-372: Replace brittle nth-child menu selectors with text-based selection for stability.Using :nth-child with WordPress admin menus is fragile across versions/configs. Prefer text-based selection.
Apply this diff to each occurrence:
- cy.get('#toplevel_page_formidable > .wp-submenu > :nth-child(8) > a').should("contain", "Form Templates").click(); + cy.contains('#toplevel_page_formidable .wp-submenu a', 'Form Templates').click();Also applies to: 391-392, 409-410, 421-422
294-296: Loosen external-content assertions to reduce flakiness.These assertions depend on remote content that can change. Exact matches (have.text) on external pages are brittle.
Apply this diff:
- cy.get('h1.margin30').should('have.text', 'Contact Us Form Template'); - cy.get('h2.aligncenter').should('contain', "What's in the Contact Us Form Template Demo"); + cy.contains('h1.margin30', 'Contact Us Form Template'); + cy.get('h2.aligncenter').should('contain', 'Contact Us Form Template');
173-175: Avoid relying on hard-coded data-id attributes; prefer content-based selectors.data-id values often vary across environments and seeds, causing false negatives. Prefer locating by visible text.
Apply this diff:
- cy.get('[data-id="20874733"] > .frm-form-templates-item-body > .frm-form-templates-item-title > .frm-form-templates-item-title-text > .frm-form-template-name').should("contain", "Payment"); - cy.get('[data-id="20874739"] > .frm-form-templates-item-body > .frm-form-templates-item-title > .frm-form-templates-item-title-text > .frm-form-template-name').should("contain", "Payment"); + cy.contains('.frm-form-templates-item-title-text .frm-form-template-name', 'Payment').should('exist'); + cy.contains('.frm-form-templates-item-title-text .frm-form-template-name', 'Payment').should('exist');If you need to assert two different “Payment” entries exist, prefer asserting count:
cy.get('.frm-form-templates-item-title-text .frm-form-template-name') .filter((_, el) => el.innerText.includes('Payment')) .should('have.length.at.least', 2);
320-320: Click the actionable element, not its SVG child, to avoid click misfires.Clicking the svg node can intermittently fail if the event isn’t bound to the child. Click the link/button itself.
Apply this diff to each occurrence:
- cy.get('a[aria-label="Close"] svg').click(); + cy.get('a[aria-label="Close"]').click();Also applies to: 369-369, 407-407, 419-419
🧹 Nitpick comments (3)
tests/cypress/e2e/Form Templates/FormTemplates.cy.js (3)
3-6: Set viewport before visiting the page for consistent layout.Viewport should be applied before cy.visit to ensure the correct responsive layout is loaded.
Apply this diff:
- cy.visit('/wp-admin/admin.php?page=formidable-form-templates'); - cy.viewport(1280, 720); + cy.viewport(1280, 720); + cy.visit('/wp-admin/admin.php?page=formidable-form-templates');
269-274: Reduce use of force: true; ensure visibility/interactivity instead.Forced clicks can hide legitimate UI issues and increase flakiness. Prefer asserting visibility/enabled state, then clicking without force.
Example pattern:
cy.get(selector).should('be.visible').and('not.be.disabled').click();Apply that in place of most .click({ force: true }) calls above unless there’s a known overlay/canvas constraint you can explicitly reason about.
Also applies to: 286-292, 300-304, 308-313, 324-330, 413-417, 425-436
293-297: Minimize dependency on external origin to stabilize tests.Cross-origin checks rely on external uptime/content and network. Prefer stubbing, or gate this behind an env flag and test the link’s href locally.
Example alternatives:
- Assert the link has expected href and rel attributes locally; skip opening external page.
- Use cy.intercept to stub the external page response if opening is essential.
Would you like me to propose a conditional test wrapper (e.g., if Cypress.env('E2E_EXTERNAL') then run external checks)?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/cypress/e2e/Form Templates/FormTemplates.cy.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cypress
- GitHub Check: Cypress
🔇 Additional comments (1)
tests/cypress/e2e/Form Templates/FormTemplates.cy.js (1)
6-7: No-op change; won’t impact failing tests.Inserting a blank line after cy.viewport has no functional effect. If the goal is to address failing tests, consider the actionable fixes below that target likely flake/failure points.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/cypress/e2e/Entries/EntriesPageDataValidations.cy.js (1)
51-57: Make CTA assertion more resilient to copy changes and punctuationExact string equality on marketing copy is brittle. Prefer a case-insensitive regex (and allow optional punctuation) to reduce flakiness across minor text edits.
Apply this diff:
cy.log("Check that the upgrade text matches one of the expected messages"); cy.get('.frm-tip-cta').then(($el) => { const text = $el.text().trim(); - expect(text).to.be.oneOf([ - "Upgrade to Pro.", - "Get 60% Off Pro!" - ]); + expect(text).to.match(/^(Upgrade to Pro\.?|Get\s*\d+% Off Pro!?)$/i); })tests/cypress/e2e/Forms/formPageDataValidation.cy.js (3)
21-21: Normalize banner text check (case-insensitive + fewer branches)Using String.match with a string literal is less explicit. Consolidate into a single, case-insensitive regex to cover all current variants and minimize maintenance.
Apply this diff:
- if (href && (text.includes('upgrading to PRO') || text.match('Get 60% Off Pro!') || text.match(/GET \d+% OFF|SAVE \d+%/))) { + if (href && /(upgrading to pro|get\s*\d+% off( pro!?)*|save\s*\d+%)/i.test(text)) {
25-31: Trim heading text and avoid exact-equality brittlenessExact equality on full H1 text is fragile (whitespace, minor copy, or typographic changes). Trim and either use contains or a regex.
Apply this diff:
- cy.get('h1').should(($h1) => { - const headingText = $h1.text(); - expect([ - 'The Only WordPress Form Maker & Application Builder Plugin', - 'Upgrade Today to Unlock the Full Power of Formidable Forms', - 'The Most Advanced WordPress Form builder' - ]).to.include(headingText); - }); + cy.get('h1').should(($h1) => { + const headingText = $h1.text().trim(); + expect(headingText).to.match( + /The Only WordPress Form Maker & Application Builder Plugin|Upgrade Today to Unlock the Full Power of Formidable Forms|The Most Advanced WordPress Form builder/ + ); + });
33-35: Minor: include href in error for easier debuggingGood switch to throwing an Error. Include href in the message for quicker triage when this trips in CI.
Apply this diff:
- } else { - throw new Error(`Unexpected banner text or missing href: "${text}"`); - } + } else { + throw new Error(`Unexpected banner text or missing href. text="${text}" href="${href}"`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/cypress/e2e/Entries/EntriesPageDataValidations.cy.js(1 hunks)tests/cypress/e2e/Forms/formPageDataValidation.cy.js(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/cypress/e2e/Forms/formPageDataValidation.cy.js (1)
js/formidable_admin.js (1)
href(10881-10881)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
tests/cypress/e2e/Forms/formPageDataValidation.cy.js (2)
24-31: Fix H1 literal case and trim; current equality is brittle and likely still failingThe newly added string has “Form builder” with a lowercase “b”. The public page uses “Form Builder”. Also, the assertion compares the full text without trimming, making it sensitive to whitespace changes.
Apply this diff to fix the case and normalize whitespace:
- cy.get('h1').should(($h1) => { - const headingText = $h1.text(); - expect([ - 'The Only WordPress Form Maker & Application Builder Plugin', - 'Upgrade Today to Unlock the Full Power of Formidable Forms', - 'The Most Advanced WordPress Form builder' - ]).to.include(headingText); - }); + cy.get('h1').invoke('text').then((t) => { + const headingText = t.trim(); + const allowed = [ + 'The Only WordPress Form Maker & Application Builder Plugin', + 'Upgrade Today to Unlock the Full Power of Formidable Forms', + 'The Most Advanced WordPress Form Builder' // fix case + ]; + expect(allowed).to.include(headingText); + });Optional hardening (more resilient to future copy tweaks):
- cy.get('h1').invoke('text').then((t) => { - const headingText = t.trim(); - const allowed = [ - 'The Only WordPress Form Maker & Application Builder Plugin', - 'Upgrade Today to Unlock the Full Power of Formidable Forms', - 'The Most Advanced WordPress Form Builder' // fix case - ]; - expect(allowed).to.include(headingText); - }); + cy.get('h1').invoke('text').then((t) => { + const headingText = t.trim(); + const patterns = [ + /the only wordpress form maker .* application builder plugin/i, + /upgrade today .* full power of formidable forms/i, + /the most advanced wordpress form builder/i, + ]; + expect(patterns.some((re) => re.test(headingText))).to.eq(true, `Unexpected H1: "${headingText}"`); + });
21-23: Make promo-banner text detection resilient (use a single case-insensitive regex)The current condition mixes exact substrings and a case-sensitive regex; minor copy or casing changes will break the test.
- if (href && (text.includes('upgrading to PRO') || text.includes( 'Get 60% Off Pro!' ) || text.match(/GET \d+% OFF|SAVE \d+%/))) { + const promoRe = /(upgrad(?:e|ing)\s+to\s+pro)|((?:get|save)\s+\d+%\s+off)/i; + if (href && promoRe.test(text)) {
♻️ Duplicate comments (1)
tests/cypress/e2e/Forms/formPageDataValidation.cy.js (1)
41-47: Harden URL comparisons using URL API (handles trailing slash on baseUrl and relative hrefs)This avoids false negatives when baseUrl has a trailing slash or the DOM uses a relative href. This was previously suggested and remains applicable.
- cy.get('a.frm-header-logo') - .should('have.attr', 'href', origin + "/wp-admin/admin.php?page=formidable") - .click(); + const expected = new URL('/wp-admin/admin.php?page=formidable', origin).toString(); + cy.get('a.frm-header-logo') + .should('have.attr', 'href') + .then(href => expect(new URL(href, origin).toString()).to.eq(expected)) + .click(); - cy.url().should('eq', origin + "/wp-admin/admin.php?page=formidable"); + cy.url().should('eq', expected);
🧹 Nitpick comments (1)
tests/cypress/e2e/Forms/formPageDataValidation.cy.js (1)
34-35: Enrich error details for faster triageInclude the href value in the thrown error to speed up debugging when the banner text doesn’t match.
- throw new Error(`Unexpected banner text or missing href: "${text}"`); + throw new Error(`Unexpected banner text or missing href: "${text}" (href: ${href ?? 'null'})`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/cypress/e2e/Forms/formPageDataValidation.cy.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
tests/cypress/e2e/Form Templates/FormTemplates.cy.js(4 hunks)tests/cypress/e2e/Forms/formPageDataValidation.cy.js(3 hunks)tests/cypress/e2e/Forms/formsSettings.cy.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/cypress/e2e/Forms/formPageDataValidation.cy.js
- tests/cypress/e2e/Form Templates/FormTemplates.cy.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/cypress/e2e/Forms/formsSettings.cy.js (1)
114-121: Optional hardening: ensure the submit isn’t disabled and DRY via a custom commandTo reduce flakiness, assert the submit control isn’t disabled before clicking. You can also DRY this repeated pattern with a Cypress custom command.
- Minimal hardening within the current blocks:
cy.get('.frm_forms', { timeout: 10000 }) .should('be.visible') .within(() => { cy.get("button[type='submit'], input[type='submit']") .filter(':visible') .first() + .should('not.be.disabled') .click(); });
- Optional: Extract a custom command (e.g., cy.submitFrmForm()) and replace both occurrences with a single call.
Add to cypress/support/commands.js:
Cypress.Commands.add('submitFrmForm', () => { cy.get('.frm_forms', { timeout: 10000 }) .should('be.visible') .within(() => { cy.get("button[type='submit'], input[type='submit']") .filter(':visible') .first() .should('not.be.disabled') .click(); }); });Then replace the inline blocks here with:
- cy.get('.frm_forms', { timeout: 10000 }) - .should('be.visible') - .within(() => { - cy.get("button[type='submit'], input[type='submit']") - .filter(':visible') - .first() - .click(); - }); + cy.submitFrmForm();Also applies to: 135-142
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/cypress/e2e/Forms/formsSettings.cy.js(2 hunks)
🔇 Additional comments (2)
tests/cypress/e2e/Forms/formsSettings.cy.js (2)
114-121: Scoped submit click within .frm_forms: solid fix to avoid stray clicksGood job scoping the submit click to the form container and selecting the first visible submit. This addresses the flakiness risk from broad page-wide selectors.
135-142: Consistent scoping applied to the “In Theme” path — looks goodMirroring the scoped submit logic here ensures both preview paths behave deterministically.
|
@Crabcyborg E2E tests are now in a more stable state. However, the PHPUnit check is failing, which I think is unrelated to Cypress. |
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
Thank you @lauramekaj1!
🙌
|
@lauramekaj1 Actually, I re-ran the tests, since there was a PHPUnit issue (I fixed that), and now I'm seeing a new e2e issue. Can you look into this one? I think the sales button text has changed. 
|
2 participants
No description provided.