Solution #172
Solution #172
Conversation
There was a problem hiding this comment.
Great work so far! 😊 However, there are a few areas that need some improvements. Please consider generating a random password instead of using a fixed one, check for more specific elements after successful registration, and avoid hardcoding URLs in your tests. 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
| describe('Sign Up page', () => { | ||
| it('should provide an ability to register new account', () => { | ||
| const { email, username, password } = generateUser(); | ||
|
|
||
| cy.visit('https://react-redux.realworld.io/#/register?_k=spysa9'); |
There was a problem hiding this comment.
Hardcoding URLs in the tests is not a good practice. If the URL changes in the future, you will have to change it in every place it's used. Consider storing the base URL in a configuration file or environment variable and just append the path specific to the test.
cypress/e2e/signUp.cy.js
Outdated
| .type(password); | ||
|
|
||
| cy.get('button[type=submit]') | ||
| .should('exist').click(); |
There was a problem hiding this comment.
The .should('exist') assertion before the click action might not be necessary. Cypress automatically waits for elements to exist before executing actions (like click). If the button doesn't exist, the test will fail anyway.
cypress/e2e/signUp.cy.js
Outdated
| cy.get('button[type=submit]') | ||
| .should('exist').click(); | ||
|
|
||
| cy.url().should('not.contain', '/register'); |
There was a problem hiding this comment.
This assertion checks that the URL does not contain '/register', but it doesn't check if the user was redirected to the correct page after registration. Consider checking that the URL matches the expected URL after successful registration.
|
|
||
| cy.get('a.nav-link').contains(username).should('exist'); |
There was a problem hiding this comment.
Although checking for the existence of a username in the navigation bar is a good way to confirm successful registration, it might be more robust to check something more specific to the user's account, like a welcome message or user settings. This would make your test more resistant to changes in the UI.
| import { faker } from '@faker-js/faker'; | ||
|
|
||
| function generateUser() { | ||
| const randomNumber = Math.random().toString().slice(2, 6); |
There was a problem hiding this comment.
Consider adding a comment here explaining the purpose of the randomNumber and why it's used in the username. This will improve the readability of your code.
cypress/support/generate.js
Outdated
| const randomNumber = Math.random().toString().slice(2, 6); | ||
| const username = faker.internet.userName() + '_' + randomNumber; | ||
| const email = `${username}@mail.com`; | ||
| const password = '12345Qwert!'; |
There was a problem hiding this comment.
Hardcoding passwords in your code can be a security risk, even for test data. Consider generating a random password for each user instead. This could also help you catch potential issues with different password inputs.
There was a problem hiding this comment.
Great work so far! 👍 However, there are a few improvements that can be made to enhance the code quality and maintainability. Please consider using more specific selectors, avoid hardcoding URLs in your tests, move the base URL to a global configuration file, and handle uncaught exceptions in a global configuration file. 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
| @@ -1,5 +1,28 @@ | |||
| import { generateUser } from '../support/generate'; | |||
|
|
|||
| Cypress.on('uncaught:exception', () => { | |||
There was a problem hiding this comment.
It's recommended to handle uncaught exceptions in a global configuration file (like commands.js or index.js in the support folder) instead of in individual test files. This improves code maintainability and readability.
cypress/e2e/signUp.cy.js
Outdated
| describe('Sign Up page', () => { | ||
| it('should provide an ability to register new account', () => { | ||
| const { email, username, password } = generateUser(); | ||
|
|
||
| cy.visit('/#/register?_k=spysa9'); |
There was a problem hiding this comment.
It's better to move the base URL ('/#/register?_k=spysa9') to a global configuration file or environment variable. This will make it easier to change the base URL when the test environment changes.
cypress/e2e/signUp.cy.js
Outdated
|
|
||
| cy.get('button[type=submit]').click(); | ||
|
|
||
| cy.url().should('contain', '/#/?_k=c1q9kl'); |
There was a problem hiding this comment.
It's better to avoid hardcoding URLs in your tests. If the URL structure changes, your tests will break. Consider using a relative URL or a URL from the application's state.
|
|
||
| cy.get('a.nav-link').contains(username).should('exist'); |
There was a problem hiding this comment.
It's better to use more specific selectors than 'a.nav-link'. This will make your tests more resilient to changes in the HTML structure. Consider adding a data-test attribute to the element you want to select.
There was a problem hiding this comment.
Great job on your pull request! Your code is generally well-written and it's great to see you using the faker library to generate random user credentials. There are a few areas that could be improved, such as adding assertions to check if registration was successful and adding comments to your tests. Keep up the good work and continue learning and improving! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| cy.get('a.nav-link').contains(username).should('exist'); |
There was a problem hiding this comment.
It's good that you are checking if the username appears on the navigation bar after registration, but you should also add some assertions to check if the registration was successful. For example, you could check if a success message appears on the screen or if the user is redirected to the correct page. This will make your tests more robust and reliable.
| describe('Sign Up page', () => { | ||
| it('should provide an ability to register new account', () => { | ||
| const { email, username, password } = generateUser(); | ||
|
|
||
| cy.visit(Cypress.config().baseUrl); | ||
|
|
||
| cy.get('[placeholder=Username]') | ||
| .type(username); | ||
|
|
||
| cy.get('[placeholder=Email]') | ||
| .type(email); | ||
|
|
||
| cy.get('[placeholder=Password]') | ||
| .type(password); | ||
|
|
||
| cy.get('button[type=submit]').click(); | ||
|
|
||
| cy.get('a.nav-link').contains(username).should('exist'); | ||
| }); |
There was a problem hiding this comment.
It's a good practice to add comments to your tests to explain what each part of the test is doing. This can make your tests easier to understand and maintain, especially for other people who might work on them in the future. Try to describe the purpose of each test, what actions it performs and what results it expects.
No description provided.