-
Notifications
You must be signed in to change notification settings - Fork 15
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
PP-10627 add agreement details page cy test #4038
PP-10627 add agreement details page cy test #4038
Conversation
a90f4de
to
e6c52dd
Compare
|
||
cy.get('.govuk-heading-l').should('have.text', 'Agreement detail') | ||
|
||
cy.get('.govuk-summary-list__value').contains('Test User') |
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.
It's not clear which .govuk-summary-list__value
this is checking, which might make it difficult to debug if it starts failiing.
To be more deliberate about which elements we are checking the contents of, we could add a data-cy
attribute on the elements in the Nunjucks and use that in the test to get the element we're checking.
Also, are we checking all the attributes on the agreement are displayed as expected? Id, reference, status etc.
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.
👍
12e6f6b
to
b02f026
Compare
|
||
cy.get('.govuk-heading-l').should('have.text', 'Agreement detail') | ||
|
||
cy.get('[data-cy=agreement-detail]').get('h2').contains('Payment instrument') |
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.
This assertion is ever so slightly confusing as we're also asserting on the "Payment instrument" title further down when we have assertions for the payment instrument section. It makes it seem like the assertions below this might be looking at the payment instrument section.
Maybe we can remove this 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.
👍
@@ -57,7 +57,7 @@ describe('Agreements', () => { | |||
cy.setEncryptedCookies(userExternalId) | |||
}) | |||
|
|||
it('should correctly display agreements for a given service', () => { | |||
it.only('should correctly display agreements for a given service', () => { |
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.
.only
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.
🤦
- add a new test that checks the rendering of the agreement details page in specifics
b02f026
to
d9a0549
Compare
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.
LGTM!
WHAT