-
Notifications
You must be signed in to change notification settings - Fork 394
feat: add Account Settings e2e continuum tets for Email, Password, Personal Details, Notifications Pages #20126
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
Conversation
…rsonal Details Pages
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.
So far only two files run AccessContinuum tests. Let's circle back once the rest is completed and existing comments have been resolved with changes applied throughout.
You may also want to check out /storefrontapp-e2e-cypress/docs/a11y-testing.md
for some reasoning behind the comments here. There should also be examples on how to specifically target modals and such.
* This test checks accessibility concerns on the Account Settings Close Account page using Access Continuum | ||
*/ | ||
describe('Account Settings / Close Account Page Accessibility', () => { | ||
isolateTests(); |
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 test isolation is on on by default, I think this may be unnecessary.
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.
Addressed.
it('initial page load', () => { | ||
updateEmail.registerAndLogin(); | ||
cy.visit(CLOSE_ACCOUNT_URL); | ||
cy.get('cx-breadcrumb h1').should('contain', 'Close Account'); |
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.
cy.get('cx-breadcrumb h1').should('contain', 'Close Account'); | |
cy.get('cx-breadcrumb h1').contains('Close Account'); |
We should not use assertions. Ideally we want failures to happen only if Access Continuum finds issues. Otherwise we are risking false negatives. This goes for all the tests in this PR.
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.
Addressed.
it('Close Account update success', () => { | ||
updateEmail.registerAndLogin(); | ||
cy.visit(CLOSE_ACCOUNT_URL); | ||
cy.get('cx-breadcrumb h1').should('contain', 'Close Account'); | ||
cy.get('cx-close-account button.btn-primary').click(); | ||
cy.get('cx-close-account-modal .cx-close-account-modal-title').should( | ||
'contain', | ||
'Confirm Account Closure' | ||
); | ||
cy.get( | ||
'cx-close-account-modal .cx-close-account-modal-footer button.btn-secondary' | ||
).click(); | ||
|
||
cy.get('main').a11yRunContinuumTest(); | ||
}); |
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.
Not sure why the modal is opened and then closed? Access Continuum can only see what's part of the DOM while the test is run.
The main
has already been tested in the previous test we should only target the modal here.
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.
Addressed.
}); | ||
|
||
it('initial page load', () => { | ||
updateEmail.registerAndLogin(); |
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.
There is no point of having Cypress manually go through the forms. We can login with a request using cy.requireLoggedIn(standardUser);
command. This should make our test run a lot faster.
If we are not making any changes to the account we should be able to just reuse one.
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.
Addressed.
const checkbox = cy | ||
.get('cx-notification-preference') | ||
.find('input.form-check-input'); | ||
// TODO: Why is check causing error!? | ||
return isEmailNotificationChannel ? checkbox.check() : checkbox.uncheck(); |
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.
You should not use variables with cypress. Trying to access sth outside of it's queue will usually cause problems. Use aliases instead. You can read more here.
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.
Addressed.
it('notification preference update success', () => { | ||
updateEmail.registerAndLogin(); | ||
cy.visit(NOTIFICATION_PREFERENCE_URL); | ||
cy.get('cx-breadcrumb h1').should('contain', 'Notification Preference'); | ||
cy.get('cx-notification-preference input[type="checkbox"]').should( | ||
'not.be.checked' | ||
); | ||
updateNotificationPreferencesForm({ isEmailNotificationChannel: true }); | ||
cy.get('cx-notification-preference input[type="checkbox"]').should( | ||
'be.checked' | ||
); | ||
|
||
cy.get('main').a11yRunContinuumTest(); | ||
}); |
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.
Are there unique elements here compared to the previous test? Looks like we are just scanning the same thing twice.
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.
Addressed.
spartacus
|
Project |
spartacus
|
Branch Review |
feature/CXSPA-9722
|
Run status |
|
Run duration | 04m 55s |
Commit |
|
Committer | Uros Lates |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
3
|
|
2
|
|
0
|
|
152
|
View all changes introduced in this branch ↗︎ |
Closes: