Sign Up #170
Sign Up #170
Conversation
There was a problem hiding this comment.
Great work so far! 👍 However, there are a few improvements needed. Please consider using more specific selectors for better maintainability and resilience of your tests. Also, remember to complete your assertions and remove any unused code to keep your files clean. Keep up the good work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
cypress/e2e/signUp.cy.js
Outdated
|
|
||
| cy.visit('user/register'); | ||
| cy.get(':nth-child(1) > .form-control').type(user.username); |
There was a problem hiding this comment.
It's recommended to use more specific selectors rather than :nth-child which can easily break if the structure of the form changes. Consider assigning data-* attributes to your form fields and use them as selectors. This will make your tests more resilient and maintainable.
cypress/e2e/signUp.cy.js
Outdated
|
|
||
| cy.visit('user/register'); | ||
| cy.get(':nth-child(1) > .form-control').type(user.username); | ||
| cy.get(':nth-child(2) > .form-control').type(user.email); |
There was a problem hiding this comment.
Similar to the above comment, consider using more specific selectors for better maintainability.
cypress/e2e/signUp.cy.js
Outdated
| cy.visit('user/register'); | ||
| cy.get(':nth-child(1) > .form-control').type(user.username); | ||
| cy.get(':nth-child(2) > .form-control').type(user.email); | ||
| cy.get(':nth-child(3) > .form-control').type(user.password); |
There was a problem hiding this comment.
Similar to the above comment, consider using more specific selectors for better maintainability.
cypress/e2e/signUp.cy.js
Outdated
| cy.get(':nth-child(1) > .form-control').type(user.username); | ||
| cy.get(':nth-child(2) > .form-control').type(user.email); | ||
| cy.get(':nth-child(3) > .form-control').type(user.password); | ||
| cy.get('.btn').click(); |
There was a problem hiding this comment.
Similar to the above comment, consider using more specific selectors for better maintainability.
cypress/e2e/signUp.cy.js
Outdated
| cy.get(':nth-child(2) > .form-control').type(user.email); | ||
| cy.get(':nth-child(3) > .form-control').type(user.password); | ||
| cy.get('.btn').click(); | ||
| cy.get(':nth-child(4) > .nav-link'); |
There was a problem hiding this comment.
This line doesn't seem to do anything. If you're trying to assert that this element exists, you should use cy.get('...').should('exist').
cypress/e2e/signUp.cy.js
Outdated
| cy.get(':nth-child(3) > .form-control').type(user.password); | ||
| cy.get('.btn').click(); | ||
| cy.get(':nth-child(4) > .nav-link'); | ||
| cy.should('contain', user.username.toLowerCase()); |
There was a problem hiding this comment.
This assertion seems to be incomplete. cy.should is not a valid command. You probably intended to use it on a previously yielded subject from a command like cy.get. If you're trying to assert that the username exists in the DOM, the correct syntax would be cy.get('yourSelector').should('contain', user.username.toLowerCase()).
| @@ -0,0 +1,11 @@ | |||
| import { faker } from '@faker-js/faker'; | |||
There was a problem hiding this comment.
It seems like you're trying to import 'faker' from '@faker-js/faker', but this package doesn't exist. You should probably import it from 'faker' package. Like this: import faker from 'faker';
| function generateUser() { | ||
| const username = faker.internet.userName(); | ||
| const email = `${username}@mail.com`; | ||
| const password = '12345Qwert!'; |
There was a problem hiding this comment.
Hardcoding the password is not a good practice, even in test data generation. It would be better to generate a random password using faker. For example, you can use faker.internet.password(); to generate a random password.
There was a problem hiding this comment.
Great job on your work so far! 👍 However, there are some improvements that need to be made. Please correct the import statement in generate.js and consider adding more validation checks in your test cases, such as checking other user details and the successful form submission. Keep going! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| describe('Sign Up page', () => { | ||
| it('should provide an ability to register new account', () => { | ||
| const user = generateUser(); |
There was a problem hiding this comment.
It would be better if you add some assertions to check if the user was generated correctly. This will ensure that the test data is correct before running the test.
| cy.get('input[placeholder="Username"]').type(user.username); | ||
| cy.get('input[type="email"]').type(user.email); | ||
| cy.get('input[type="password"]').type(user.password); | ||
| cy.get('button[type="submit"]').click(); |
There was a problem hiding this comment.
After clicking the submit button, it is a good practice to check if the form was submitted successfully. You can add a check to see if the page has been redirected to the expected URL.
| cy.get('input[type="email"]').type(user.email); | ||
| cy.get('input[type="password"]').type(user.password); | ||
| cy.get('button[type="submit"]').click(); | ||
| cy.get('.nav-link').should('contain', user.username.toLowerCase()); |
There was a problem hiding this comment.
This line checks if the username appears in the navigation bar, which is good. However, it would be better if you also check other user details, such as email, to ensure that the user was registered correctly.
| @@ -0,0 +1,11 @@ | |||
| import { faker } from '@faker-js/faker'; | |||
There was a problem hiding this comment.
You have used import statement which is not supported in CommonJS modules. You should use require instead. Try replacing this line with const faker = require('@faker-js/faker');.
There was a problem hiding this comment.
Great job on your pull request. Your Cypress configuration is set up correctly, and your tests are well-structured and logical. However, there are a few improvements that can be made to make your tests more robust and maintainable. Please review the comments for detailed feedback. Keep up the good work! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| cy.visit('user/register'); |
There was a problem hiding this comment.
It's a good practice to store URLs, selectors and other constants in a separate file or at least at the top of your test file. This way, if something changes, you only have to update it in one place.
| cy.get('input[type="email"]').type(user.email); | ||
| cy.get('input[type="password"]').type(user.password); | ||
| cy.get('button[type="submit"]').click(); | ||
| cy.url().should('include', '/'); |
There was a problem hiding this comment.
This assertion only checks if the '/' is included in the URL. It would be more robust to check if the URL is exactly what you expect after a successful registration.
| cy.get('input[type="password"]').type(user.password); | ||
| cy.get('button[type="submit"]').click(); | ||
| cy.url().should('include', '/'); | ||
| cy.get('.nav-link').should('contain', user.username.toLowerCase()); |
There was a problem hiding this comment.
This assertion checks if the username is displayed in the navigation bar after registration. However, it might be more accurate to check for the exact location of the username in the DOM, not just that it contains the username.
| @@ -0,0 +1,11 @@ | |||
| import { faker } from '@faker-js/faker'; | |||
There was a problem hiding this comment.
It seems like you're trying to import faker from @faker-js/faker, but this module doesn't exist. The correct module name is faker. So, the correct import statement will be import faker from 'faker';.
| function generateUser() { | ||
| const username = faker.internet.userName(); | ||
| const email = `${username}@mail.com`; | ||
| const password = '12345Qwert!'; |
There was a problem hiding this comment.
Hardcoding the password here may not be a good idea. It would be better to generate a random password using faker.js. You can do this with faker.internet.password();.
No description provided.