<fix errors>#81
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The tooling infrastructure is correctly set up, but there are critical functional bugs that must be fixed. HomePage.js line 10 has a method named assertYourFeedTabIsVisible that uses toBeHidden() instead of toBeVisible() — after successful sign-in/sign-up, the 'Your Feed' tab should be visible, not hidden. This bug will cause all positive authentication tests to fail (signInPositive.spec.js line 25 and signUpPositive.spec.js line 28 expect the tab to be visible). Additionally, three test files contain empty destructured parameters async ({}) that trigger ESLint no-unused-vars errors: signInNegative.spec.js, signUpNegative.spec.js, and signUpPositive.spec.js. Remove the empty braces or use the destructured values to resolve these ESLint warnings. The ESLint configuration structure itself looks correct with the flat config format.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| this.signUpButton = page.getByRole('button', { name: 'Sign up' }); | ||
| this.errorMessage = page.getByRole('list').nth(1); |
There was a problem hiding this comment.
The method name is 'assertYourFeedTabIsVisible' but it calls 'toBeHidden()'. According to the signInPositive.spec.js test, after successful sign-in the 'Your Feed' tab should be VISIBLE, not hidden. This assertion will cause test failures.
| this.passwordField = page.getByPlaceholder('Password'); | ||
| this.signInButton = page.getByRole('button', { name: 'Sign in' }); | ||
| this.errorMessage = page.getByRole('list').nth(1); | ||
| } |
There was a problem hiding this comment.
The assertion checks if the tab is hidden (toBeHidden()), but based on the test flow (signing in should make the user's feed visible), this should likely be toBeVisible() instead. A user feed tab would be expected to appear after successful authentication.
| async assertYourFeedTabIsVisible() { | ||
| await expect(this.yourFeedTab).toBeVisible(); | ||
| await expect(this.yourFeedTab).toBeHidden(); |
There was a problem hiding this comment.
The method is named assertYourFeedTabIsVisible but uses toBeHidden() assertion. After successful authentication, the 'Your Feed' tab should be visible, not hidden. This should be toBeVisible() to match the method name and test intent.
| { languageOptions: { globals: globals.node } }, | ||
| { |
There was a problem hiding this comment.
The assertion checks for .toBeHidden(), but after successful sign-in the 'Your Feed' tab should be VISIBLE. This assertion is logically inverted and will cause test failures.
| }, | ||
| eslintConfigPrettier, | ||
| ]; | ||
|
|
There was a problem hiding this comment.
Empty destructuring {} is an unused parameter. Consider removing it or using the destructured value.
| `username:Username must start with a letter, have no spaces, and be 2 - 40 characters.`, | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Empty destructured parameter '{}' contains no destructured values - this is unnecessary and will trigger ESLint no-unused-vars warnings.
| await signInPage.open(); | ||
| }); |
There was a problem hiding this comment.
This bug causes the positive tests to fail. After successful authentication, the 'Your Feed' tab should be VISIBLE, not hidden. The assertion should be .toBeVisible() to match the test expectation on line 25 of signInPositive.spec.js and line 28 of signUpPositive.spec.js.
| let user; | ||
| test.beforeEach( async ({ page }) => { | ||
| signUpPage = new SignUpPage(page); | ||
| homePage = new HomePage(page); | ||
|
|
There was a problem hiding this comment.
This method is named 'assertYourFeedTabIsVisible' but calls 'toBeHidden()'. The test expectations (signInPositive.spec.js line 25, signUpPositive.spec.js line 28) show that after successful authentication, the 'Your Feed' tab should be visible, not hidden. This assertion is logically inverted and will cause tests to fail.
| username: `${faker.person.firstName()}_${faker.person.lastName()}`, | ||
| email: faker.internet.email(), | ||
| password: faker.internet.password(), | ||
| }; |
There was a problem hiding this comment.
Empty destructuring '{}' is an unused parameter that will trigger ESLint error for 'no-unused-vars'. Either remove the destructuring or use the destructured value.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. All previously identified critical bugs have been successfully resolved: HomePage.js now correctly uses toBeVisible() for the assertYourFeedTabIsVisible method, and all test files (signInNegative.spec.js, signUpNegative.spec.js, signUpPositive.spec.js) properly destructure { page } from the test context, eliminating the ESLint no-unused-vars errors. The ESLint configuration is correctly structured using the flat config format with appropriate plugins and rules. All task requirements are met and the implementation is complete.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
add new plugins: ESLint, Prettier, Husky and Lint-Staged
and use them to corrected 10/10 errors