Add tests #281
Add tests #281
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request. These issues mainly revolve around redundant code, such as duplicate statements and hooks, which can lead to confusion and potential errors. Additionally, there are structural issues like misplaced hooks and incomplete test descriptions that need attention. Please review the detailed feedback and make the necessary changes to ensure clarity and functionality. Keep up the great work, and don't hesitate to reach out if you have any questions! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
| describe('Article', () => { | ||
| before(() => { | ||
| let article; |
There was a problem hiding this comment.
The article variable is declared here but not initialized. Ensure that the generateArticle task successfully assigns a value to it before it is used in tests.
cypress/e2e/article.cy.js
Outdated
| before(() => { | ||
| cy.task('generateArticle').then((generatedArticle) => { | ||
| article = generatedArticle; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Consider adding error handling for the generateArticle task to ensure that article is properly initialized before proceeding with the tests. This will help prevent potential issues if the task fails.
cypress/e2e/settings.cy.js
Outdated
| @@ -5,28 +5,11 @@ describe('Settings page', () => { | |||
| before(() => { | |||
|
|
|||
| }); | |||
| import SettingsPageObject from '../support/pages/settings.pageObject'; | |||
There was a problem hiding this comment.
The import statement for SettingsPageObject should be placed at the top of the file, outside of any hooks or describe blocks, to ensure it is available throughout the test file.
cypress/e2e/settings.cy.js
Outdated
| beforeEach(() => { | ||
| const settingsPage = new SettingsPageObject(); | ||
|
|
||
| }); | ||
|
|
||
| it('should provide an ability to update username', () => { | ||
|
|
||
| }); | ||
|
|
||
| it('should provide an ability to update bio', () => { | ||
|
|
||
| }); | ||
|
|
||
| it('should provide an ability to update an email', () => { | ||
|
|
||
| }); | ||
|
|
||
| it('should provide an ability to update password', () => { | ||
|
|
||
| }); | ||
|
|
||
| it('should provide an ability to log out', () => { | ||
|
|
||
| }); | ||
| }); | ||
| describe('Settings page', () => { | ||
| cy.task('db:clear'); | ||
| }); |
There was a problem hiding this comment.
The beforeEach hook should be placed inside the describe block to ensure it runs before each test case. Additionally, the describe block is duplicated, which is incorrect. Ensure there is only one describe block for the 'Settings page'.
cypress/e2e/signIn.cy.js
Outdated
| before(() => { | ||
| user = generateUser; | ||
| cy.task('generateUser').then((generatedUser) => { | ||
| user = generatedUser; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
There is a duplicate before hook and generateUser task. Remove the redundant before hook and ensure that the user variable is initialized only once. This will help avoid confusion and potential errors in the test setup.
| } | ||
|
|
||
| get passwordField() { | ||
| return cy.getByDataCy('password-sign-in'); | ||
| return this.getByDataCy('password-sign-in'); |
There was a problem hiding this comment.
There is a redundant return statement here. Remove the duplicate return this.getByDataCy('password-sign-in'); to avoid confusion and potential errors.
| } | ||
|
|
||
| get signInBtn() { | ||
| return cy.getByDataCy('sign-in-btn'); | ||
| return this.getByDataCy('sign-in-btn'); |
There was a problem hiding this comment.
There is a redundant return statement here. Remove the duplicate return this.getByDataCy('sign-in-btn'); to avoid confusion and potential errors.
| } | ||
|
|
||
| typeEmail(email) { | ||
| this.emailField | ||
| .type(email); | ||
| .type(email); |
There was a problem hiding this comment.
There is a redundant type statement here. Remove the duplicate .type(email); to ensure the method is clear and concise.
| } | ||
|
|
||
| typePassword(password) { | ||
| this.passwordField | ||
| .type(password); | ||
| .type(password); |
There was a problem hiding this comment.
There is a redundant type statement here. Remove the duplicate .type(password); to ensure the method is clear and concise.
| } | ||
|
|
||
| clickSignInBtn() { | ||
| this.signInBtn | ||
| .click(); | ||
| .click(); |
There was a problem hiding this comment.
There is a redundant click statement here. Remove the duplicate .click(); to ensure the method is clear and concise.
There was a problem hiding this comment.
Your code meets the task requirements and constraints, covering essential functionalities with well-structured end-to-end tests. You've effectively utilized the Page Object Model and data generation techniques, which is commendable. However, there are a few areas for improvement:
- Duplicate Code: There are several instances of duplicate lines, such as in the
emailFieldgetter,assertHeaderContainUsernamemethod, andusernameLinkgetter. Removing these redundancies will enhance code clarity and prevent potential errors. - Misplaced Blocks: Ensure that
describeanditblocks are correctly placed and complete. Misplaced or incomplete blocks can disrupt test execution. - Command Clarity: Verify that custom commands like
cy.registerare defined and imported correctly to avoid confusion. - Error Handling: Improve error messages in
catchblocks for better debugging context.
These issues should be addressed to improve code quality and maintainability. Keep up the good work, and use this feedback to refine your skills further. Your dedication to learning is evident, and addressing these points will help you grow as a developer. 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| .catch((error) => { | ||
| throw new Error("Failed to generate article: " + error); |
There was a problem hiding this comment.
The error handling in the catch block is functional, but the error message could be more descriptive. Consider including more context about the error, such as the specific step or data involved, to aid in debugging.
| }); | ||
|
|
||
| beforeEach(() => { | ||
| const settingsPage = new SettingsPageObject(); |
There was a problem hiding this comment.
The settingsPage object is being re-declared inside beforeEach, which is unnecessary since it is already declared at the top of the describe block. Consider removing this line to avoid redundancy.
| describe('Settings page', () => { | ||
| cy.task('db:clear'); | ||
| }); |
There was a problem hiding this comment.
There is a misplaced describe block inside the beforeEach hook. This is likely a mistake, as describe blocks should not be nested inside hooks. Remove this block to maintain the correct test structure.
| signInPage.typeEmail(user.email); | ||
| signInPage.typePassword(user.password); | ||
| signInPage.clickSignInBtn(); | ||
|
|
||
| homePage.assertHeaderContainUsername(user.username); | ||
| @@ -29,6 +29,12 @@ describe('Sign In page', () => { |
There was a problem hiding this comment.
The describe block appears to be misplaced. It should wrap the entire set of tests, including the before hook and all it blocks. Ensure that the describe block is correctly placed to encapsulate the tests logically.
| }); | ||
| }); | ||
|
|
||
| it('should provide an ability to log in with existing credentials', () => { | ||
| signInPage.visit(); | ||
| cy.register(user.email, user.username, user.password); |
There was a problem hiding this comment.
The cy.register command is used here, but it is not clear if this command is defined or imported. Ensure that cy.register is a valid Cypress command or custom command defined elsewhere in your test suite.
|
|
||
| cy.contains('Your account has been created').should('be.visible'); | ||
| }); | ||
|
|
||
| it('should ...', () => { |
There was a problem hiding this comment.
There is an incomplete it block declaration. It seems like a placeholder or a mistake. Ensure that each test case has a complete and descriptive title, or remove this line if it's not needed.
|
|
||
| cy.contains('Following').should('be.visible'); // Verify that the user is followed | ||
| }); | ||
|
|
||
| it.skip('should be able to follow the another user', () => { |
There was a problem hiding this comment.
There is a misplaced and incomplete it block declaration. It seems to be a duplicate or a mistake. Ensure that each test case is correctly declared and remove this line if it's not needed.
| @@ -5,12 +5,12 @@ class HomePageObject extends PageObject { | |||
|
|
|||
| get usernameLink() { | |||
| return cy.getByDataCy('username-link'); | |||
| return this.getByDataCy('username-link'); | |||
There was a problem hiding this comment.
There is a duplicate return statement in the usernameLink getter. Remove the second return this.getByDataCy('username-link'); to avoid redundancy and potential errors.
| } | ||
|
|
||
| assertHeaderContainUsername(username) { | ||
| this.usernameLink | ||
| .should('contain', username); | ||
| .should('contain', username); |
There was a problem hiding this comment.
There is a duplicate line in the assertHeaderContainUsername method. Remove the second .should('contain', username); to avoid redundancy.
| @@ -5,30 +5,25 @@ class SignInPageObject extends PageObject { | |||
|
|
|||
| get emailField() { | |||
| return cy.getByDataCy('email-sign-in'); | |||
| return this.getByDataCy('email-sign-in'); | |||
There was a problem hiding this comment.
There is a duplicate return statement in the emailField getter. Remove the second return this.getByDataCy('email-sign-in'); to avoid redundancy and potential errors.
No description provided.