-
-
Notifications
You must be signed in to change notification settings - Fork 180
Cypress e2e test for Stepper component
#308
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
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.
Getting there!
I have left comments inline. Another thing : could you change the step selectors from step-0 etc to 'FS'/'BY' etc ? This will make the tests easier to read, as we will see what kind of step we are interacting with.
cypress/integration/Stepper._spec.js
Outdated
| cy.get('select').select('CC0 1.0') | ||
| cy.get('.recommended-card').should('be.visible').contains('CC0 1.0 Universal') | ||
| cy.get('.next-button').click() | ||
| cy.get('.v-checkbox:nth-child(1) > input').check() |
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.
Can you add a check for and the total number of steps should be 4.
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.
@obulat I have a question that whether total number of steps are required for the test which you have mentioned or for each and every test in the test file.
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 for this test only.
cypress/integration/Stepper._spec.js
Outdated
| .check() | ||
| cy.get('.next-button').click() | ||
| cy.get('.step-1') | ||
| .find('[value="yes"]') |
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.
Move the recommended-card check from line 83 here.
cypress/integration/Stepper._spec.js
Outdated
| cy.get('.step-1') | ||
| .find('[value="yes"]') | ||
| .check() | ||
| cy.get('.next-button').click() |
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.
Move the .recommended-card check from line 107 here.
cypress/integration/Stepper._spec.js
Outdated
| cy.get('.next-button').click() | ||
| cy.get('.step-1') | ||
| .find('[value="yes"]') | ||
| .check() |
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.
Can you add the check for .recommended-card value here?
Don't remove the check from line 186.
cypress/integration/Stepper._spec.js
Outdated
| describe('Checks whether the user can click on Start Again Button and application gets a hot reload', () => { | ||
| it('Clicks on Start Again Buttton', () => { | ||
| cy.get('.restart-button').click() | ||
| cy.get('.step-1').should('have.class','inactive') |
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.
Please, add the check for .recommended-card and .license-use-card. They shouldn't be visible.
cypress/integration/Stepper._spec.js
Outdated
| cy.get('.recommended-card').should('be.visible').contains('CC BY-ND 4.0') | ||
| cy.get('.license-use-card').should('be.visible').contains('CC BY-ND 4.0') | ||
| cy.get('.next-button').click() | ||
| cy.get('.license-use-card').scrollIntoView() |
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 do not need to scroll an element into view. In fact, when you click the 'Done' button, it should scroll automatically, and we need to check if it does.
Actually, it is better to test it on a mobile viewport. Could you set up this test with the mobile viewport?
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.
@obulat You want me to convert viewport or this particular test or for the complete test file.
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.
Yes, I would like this particular test to have a different viewport.
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.
Shall I give iphone 10 viewport. Will that be okay or any other .
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.
Any viewport width of under 768px will do, as that's the breakpoint when the two-column layout becomes a one-column layout.
|
@obulat Plzzzz have a look at this comment.At the end the repeated |
Stepper component
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 looks pretty good! However, I see a lot of spelling mistakes. Please run through with a spell checker.
Also, it might be useful to write some custom cypress commands for the really common actions, like cy.get(".next-button").click();, which could become something like:
// https://docs.cypress.io/api/cypress-api/custom-commands.html#Parent-Commands
Cypress.Commands.add('clickNext', () => {
cy.get('.next-button').click()
})
@zackkrida Okk let me do the required refactoring then. |
You don't have to do this, just the typos before merging. Up to you! |
|
@zackkrida Actually I was referring the typos as refactoring process.My bad . |
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.
Thank you for your contribution, @Cronus1007 !
I've added some refactoring to the tests.
Fixes
Fixes #302 by @obulat
Description
Technical details
Tests
Screenshots
Checklist
Update index.md).mainormaster).visible errors.
Developer Certificate of Origin
Developer Certificate of Origin