-
-
Notifications
You must be signed in to change notification settings - Fork 449
Updated cypress test #4975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Updated cypress test #4975
Conversation
- requires updated sample data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Refactors Cypress end-to-end tests by modularizing route definitions, introducing shared helpers, and updating tests to the new structure.
- Split monolithic route config into granular backend/ frontend path modules.
- Added reusable helpers for page checks, clicks, and message assertions; updated tests to use them.
- Minor admin template improvement: added title attribute to “Add New Template” button.
Reviewed Changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
cypress/support/openmage/frontend/paths.js | New frontend route map (replaces frontend part of old config). |
cypress/support/openmage/config/paths.js | Removed monolithic routes file; functionality split across backend/frontend modules. |
cypress/support/openmage/backend/system.js | New modular system routes (cache, design, email, myaccount, indexes, stores, variables, config). |
cypress/support/openmage/backend/sales.js | New modular sales routes (credit memo, invoice, order, shipment, transactions). |
cypress/support/openmage/backend/promo.js | New modular promo routes (catalog rules, quote rules). |
cypress/support/openmage/backend/newsletter.js | New modular newsletter routes (templates, queue, subscribers, reports). |
cypress/support/openmage/backend/dashboard.js | New dashboard route module. |
cypress/support/openmage/backend/customer.js | New customer routes (customers, groups, online). |
cypress/support/openmage/backend/cms.js | New CMS routes (blocks, pages, widgets) plus CMS-specific helpers. |
cypress/support/openmage/backend/catalog.js | New catalog routes (products, categories, search, sitemap, URL rewrite). |
cypress/support/openmage.js | Added utilities: check.pageElements, tools.clickAction/clickGridRow, message helpers; removed validation.saveAction. |
cypress/support/e2e.js | Switched imports to new modular backend/ frontend paths. |
cypress/support/commands.js | Updated adminGoToTestRoute signature; deprecated adminTestRoute; minor behavior changes. |
cypress/fixtures/example.json | Added sample fixture data. |
cypress/e2e/... (many backend tests) | Updated tests to use new route objects and helpers; added index/edit/new route coverage per area. |
cypress/e2e/openmage/frontend/newsletter-subscribe.cy.js | Updated to use new helpers and frontend path map. |
cypress/e2e/openmage/frontend/customer/account/create.cy.js | Updated to use new helpers and message assertions. |
app/design/adminhtml/default/default/template/newsletter/template/list.phtml | Added title attribute to "Add New Template" button for better UX and test targeting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 84 out of 84 changed files in this pull request and generated 11 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 84 out of 84 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
cypress/support/openmage.js:1
- validateFields builds the advice selector using fields[field].css, but fields[field] is a selector string; this results in '#advice-undefined-'. Use validation.css instead to correctly target the validation message.
cy.openmage = {};
const base = { | ||
_button: '.form-buttons button' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The _button selector constant is duplicated across many support files. Extract a shared selector (e.g., cy.openmage.selectors.baseButton or a common module) to avoid drift and simplify future updates.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 84 out of 84 changed files in this pull request and generated 4 comments.
cy.get(test._button).filter(':visible').should('have.length', Object.keys(path.__buttons).length); | ||
|
||
for (const button of Object.keys(path.__buttons)) { | ||
cy.get(path.__buttons[button]).should('exist'); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asserting the exact count of visible buttons can be brittle when themes/extensions add buttons. Recommend removing the length assertion and only asserting the presence of each configured button to avoid false negatives.
Copilot uses AI. Check for mistakes.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 84 out of 84 changed files in this pull request and generated 3 comments.
click: (selector, log = 'Clicking on something') => { | ||
cy.log(log); | ||
cy.get(selector).first().click({force: true, multiple: false}); | ||
}, | ||
clickContains: (element, selector, content, log = 'Clicking on some content') => { | ||
cy.log(log); | ||
cy.get(element).contains(selector, content).first().click({force: true, multiple: false}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both click helpers default to force: true, which can mask visibility/overlap issues and make tests flaky. Consider removing force by default, adding a wait for visibility (e.g., .should('be.visible')), or allowing force to be an explicit optional parameter to these helpers.
click: (selector, log = 'Clicking on something') => { | |
cy.log(log); | |
cy.get(selector).first().click({force: true, multiple: false}); | |
}, | |
clickContains: (element, selector, content, log = 'Clicking on some content') => { | |
cy.log(log); | |
cy.get(element).contains(selector, content).first().click({force: true, multiple: false}); | |
click: (selector, log = 'Clicking on something', force = false) => { | |
cy.log(log); | |
cy.get(selector).first().should('be.visible').click({force, multiple: false}); | |
}, | |
clickContains: (element, selector, content, log = 'Clicking on some content', force = false) => { | |
cy.log(log); | |
cy.get(element).contains(selector, content).first().should('be.visible').click({force, multiple: false}); |
Copilot uses AI. Check for mistakes.
cy.openmage.check = { | ||
buttons: (test, path, log = 'Checking for existing buttons') => { | ||
cy.log(log); | ||
if (path.__buttons !== undefined) { | ||
cy.get(test._button).filter(':visible').should('have.length', Object.keys(path.__buttons).length); | ||
|
||
for (const button of Object.keys(path.__buttons)) { | ||
cy.get(path.__buttons[button]).should('exist'); | ||
}; | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exact count assertion on visible buttons is brittle and may fail when the UI renders extra action buttons (e.g., from extensions or additional core actions). Since you already assert existence for each configured selector below, consider dropping the length check or relaxing it (e.g., >=) to avoid false negatives.
Copilot uses AI. Check for mistakes.
const base = { | ||
_button: '.form-buttons button', | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The base pattern (const base = { _button: '...' }) is duplicated across many support modules. Extracting a shared base (e.g., support/openmage/base.js) and importing it would reduce duplication and keep selectors consistent across pages.
const base = { | |
_button: '.form-buttons button', | |
} | |
import { base } from '../../base.js'; |
Copilot uses AI. Check for mistakes.
Description (*)
Refactored all tests.
Basic Validation
Tests
... see test files ...
Current Work
How to use
ddev cypress-open -C .cypress.config.js
(browser), orddev cypress-run -C .cypress.config.js
(cli)Structure
Every page has a file for tests and a configuration file. E.g.
cypress/e2e/openmage/backend/cms/page.cy.js
cypress/support/openmage/backend/cms/page.js