Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your submission! You've done a great job setting up the test suite and covering the main success and failure scenarios.
However, this requires changes because the test coverage is incomplete for several key validation rules from the task description. To ensure the validateRegisterForm function is fully tested, please add test cases for the following scenarios:
- Password validation:
- A password longer than 16 characters.
- A password missing a required uppercase letter.
- A password missing a required special character.
- Email validation:
- An email missing the
@symbol. - An email with invalid period placement (e.g.,
test.@example.com).
- An email missing the
Adding these tests is essential to confirm that the validation logic handles all required cases correctly. I look forward to seeing your updated submission!
✨ 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
| it(`should return error for valid email and too short password`, () => { | ||
| const result = validateRegisterForm('valid@email.com', 'P@s1!'); | ||
|
|
||
| expect(result.code).toBe(422); | ||
| expect(result.message).toBe('Password is invalid.'); | ||
| }); |
There was a problem hiding this comment.
Your tests cover many important cases! To make the suite more comprehensive, consider adding tests for these two password requirements from the description:
- Maximum length: A test for a password longer than 16 characters is missing.
- Cyrillic characters: A test with a valid password that includes Cyrillic letters would confirm that requirement is met.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great progress on adding more tests! The coverage is much more comprehensive now. However, this submission requires a few changes to be complete.
One of your test cases for an 'invalid email and valid password' uses the password 'P@ssword1', which is actually invalid because it's missing a special character. Please update it to use a password that meets all criteria, such as 'P@ssword1!'.
Additionally, to make the test suite fully cover the requirements, please add tests for the following scenarios:
- A valid password containing Cyrillic letters (e.g.,
'Пароль!1'). - An email starting with a dot (e.g.,
.test@example.com). - An email with consecutive dots (e.g.,
test..dots@example.com).
Once these are addressed, your solution will be ready. Keep up the great work!
✨ 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
| it(`should return error for invalid email and valid password`, () => { | ||
| const invalidEmail = validateRegisterForm( | ||
| 'test@com', | ||
| 'P@ssword1' | ||
| ); | ||
|
|
||
| expect(invalidEmail.code).toBe(422); | ||
| expect(invalidEmail.message).toBe('Email is invalid.'); | ||
| }); |
There was a problem hiding this comment.
This test case appears to follow an example from the task description, but the password 'P@ssword1' is actually invalid because it's missing a special character, according to the requirements.
To properly test the scenario for an 'invalid email and valid password', please update this test to use a password that meets all the criteria, such as 'P@ssword1!'.
| expect(invalidDot.message).toBe('Email is invalid.'); | ||
| } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Excellent work on adding more tests! The coverage is much more comprehensive now. To make it complete, please consider adding test cases for these remaining scenarios from the requirements:
- A valid password containing Cyrillic letters (e.g.,
'Пароль!1'). - An email starting with a dot (e.g.,
.test@example.com). - An email with consecutive dots (e.g.,
test..dots@example.com).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on this task! I am approving your solution so you can continue to the next stage. This is an educational approval, as there's one small correction needed.
You've done an excellent job adding the new test cases for email validation, which makes the test suite much stronger. The only thing to fix is the test for passwords with Cyrillic letters in src/validateRegisterForm.test.js. The current password, 'P@ssword!1A', only contains Latin characters and doesn't actually test the requirement. To fix this, please update it to include Cyrillic letters, for example: 'Пароль!1A'.
Keep up the fantastic effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| it('should accept valid password containing Cyrillic letters', () => { | ||
| const result = validateRegisterForm( | ||
| 'valid@email.com', | ||
| 'P@ssword!1A' |
There was a problem hiding this comment.
This test case is intended to verify that passwords with Cyrillic letters are accepted, but the password 'P@ssword!1A' only contains Latin characters. To properly test this requirement, please use a password that includes Cyrillic letters, for example, 'Пароль!1A'.
2 participants
No description provided.