Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a great start on the test suite, and your existing tests are well-written. However, changes are needed because a few key requirements are not yet covered. Please add tests for a valid password containing Cyrillic letters (e.g., 'Пароль1!') and for invalid emails where a dot is at the end ('test@mail.com.') or at the beginning of the top-level domain ('test@.mail.com'). Adding these tests will ensure the function is fully validated against all specifications.
✨ 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
|
|
||
| expect(isValid.code).toBe(422); | ||
| expect(isValid.message).toBe("Email is invalid."); | ||
| }); |
There was a problem hiding this comment.
Excellent tests for invalid dot usage in emails. To make the coverage complete, please also add tests for two other scenarios mentioned in the requirements:
- An email that ends with a dot (e.g.,
'test@mail.com.'). - A top-level domain that starts with a dot (e.g.,
'test@.mail.com').
|
|
||
| expect(invalidPassword.code).toBe(422); | ||
| expect(invalidPassword.message).toBe("Password is invalid."); | ||
| }); |
There was a problem hiding this comment.
Good coverage of invalid password cases. One requirement is missing a test: that passwords can include Cyrillic letters (Aa-Яя). Please add a test case for a valid password that contains Cyrillic characters to ensure full compliance.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job on adding the new test cases from the last review! The test suite is much more comprehensive. I am requesting one change before this can be approved.
In src/validateRegisterForm.test.js, the test case for an email ending with a dot is currently expecting a success (code: 200). According to the requirements, this is an invalid email format. Please update the test to assert that it returns an error with { code: 422, message: 'Email is invalid.' }.
Additionally, the build is failing due to a max-len linting error. You can fix this by shortening the long lines as suggested in the check run logs. Once the test logic is corrected, we'll be all set!
✨ 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
| expect(isValid.message).toBe("Email is invalid."); | ||
| }); | ||
|
|
||
| it(`success with an email that ends with a dot `, () => { |
There was a problem hiding this comment.
This test case is asserting a success scenario, but the requirements state that an email cannot end with a dot. Please update the test description and the assertions to check for an invalid email (code: 422 and message: 'Email is invalid.').
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on building out such a comprehensive test suite! I am approving this submission as an educational override to help you move forward, but there are a couple of important items from the previous review that still need your attention. The test case for an email ending with a dot (.) is still expecting a successful validation; this should be updated to expect an error response: { code: 422, message: 'Email is invalid.' }. Additionally, the build is failing because two lines in src/validateRegisterForm.test.js (lines 75 and 127) are too long. Please shorten the descriptions in those it() blocks to resolve the linting errors. Fixing these two points will make your solution perfect. Keep up the great effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| it(`success with an email that ends with a dot `, () => { | ||
| const isValid = validateRegisterForm("test@mail.com.", "P@ssword1!"); | ||
|
|
||
| expect(isValid.code).toBe(200); | ||
| expect(isValid.message).toBe("Email and password are valid."); | ||
| }); |
There was a problem hiding this comment.
This test case is still expecting a successful validation for an email that ends with a dot (.). According to the requirements, a dot cannot be the last character of the email. Please update this test to expect an error response: { code: 422, message: 'Email is invalid.' }.
2 participants
No description provided.