-
Notifications
You must be signed in to change notification settings - Fork 199
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
solution #171
base: main
Are you sure you want to change the base?
solution #171
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.
Great job! Your code is generally well-written and follows best practices. However, there are a few areas for improvement. Don't forget to generate unique passwords for each user and ensure the user is redirected to the correct page after registration. Also, try to use more specific selectors and move the base URL to the configuration file. Keep up the good work! 👍
✨ 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(); | ||
|
||
cy.visit('https://react-redux.realworld.io'); |
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 a good practice to move base URL to the cypress.json configuration file. This way, you don't have to repeat the same URL in every test and it's easier to maintain.
cy.get('[placeholder=Password]') | ||
.type(user.password); | ||
|
||
cy.get('.btn').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.
It would be better if the selector was more specific. '.btn' is a very generic class that could be used in many places on a page. If the structure of the page changes, this could lead to the test clicking on the wrong button.
|
||
cy.get('a.nav-link') | ||
.should('contain', user.username); |
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 a good practice to assert that the user was redirected to the correct page after the registration. You could check the URL with cy.url().should('include', '/expectedPath')
.
@@ -0,0 +1,12 @@ | |||
import { faker } from '@faker-js/faker'; |
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 seems like you're trying to import the faker
module from '@faker-js/faker', but the correct package name is 'faker'. Please update the import statement to import faker from 'faker';
.
const randomNumber = Math.random().toString().slice(2, 6); | ||
const username = faker.internet.userName() + '_' + randomNumber; | ||
const email = `${username}@mail.com`; |
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're generating a random number and appending it to the username and email. This is a good practice to ensure unique values. However, it might be a good idea to also append this random number to the password to ensure unique and hard-to-guess passwords.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The password '12345Qwert!' is hardcoded. It's a good practice to generate random and unique passwords for each user. You can use the faker.internet.password()
method to generate a random password.
No description provided.