js_check_password by Alex-redman · Pull Request #156 · mate-academy/js_check_password

Alex-redman · 2025-10-03T15:08:32Z

No description provided.

mateacademy-ai-mentor

Review Summary

Decision: Requesting changes to the tests in src/checkPassword.test.js.

Your suite is close, but there are critical blockers that contradict the specification and leave required behaviors untested.

Critical: The test labeled “valid password with 8 characters” uses P@ssw0rd! (9 chars). Either change the test string to a true 8-character valid example (e.g., P@ssw0r1, verify length and categories) or update the description to match the 9-char string.
Critical: The test expecting false for P@SSW0RD! (no lowercase) conflicts with the spec, which does not require lowercase. Update the expectation to true or remove the test.
Critical: Missing required cases. Please add:
- A failing test for missing digit (e.g., expect(checkPassword('Password!')).toEqual(false)).
- A passing test for exactly 16 characters (e.g., P@ssw0rd!P@ssw0r, verify length == 16).
- Example cases from the description: checkPassword('Password1!') === true, checkPassword('qwerty') === false, checkPassword('Str@ng') === false.
- Rejection of non‑Latin letters (e.g., Pässw0rd! and Рassword1! should be false).
- Edge cases: minimal valid with exactly one of each required category (e.g., A1!aaaaa → true), acceptance with multiple specials/digits, and invalid when uppercase+special but no digit (e.g., Passw@rd → false).

Minor: Ensure test names match the actual strings and clearly state what boundary is being validated.

Once these are addressed, the suite will accurately enforce the spec. Good work on the existing structure and many useful checks—these additions will make it complete and correct.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-10-03T15:12:35Z

src/checkPassword.test.js

  });

  it(`should return 'true' for the valid password with 8 characters`, () => {
+    expect(checkPassword('P@ssw0rd!')).toEqual(true);


The test description says "valid password with 8 characters" but the assertion uses 'P@ssw0rd!' which is 9 characters. Either change the expectation string to a true 8-character valid example (for instance, something like 'P@ssw0r1' — verify it has 8 chars and contains an uppercase, digit and special) or update the test description to match the string used.

mateacademy-ai-mentor · 2025-10-03T15:12:35Z

src/checkPassword.test.js

+  it(`should return 'false' when password without small letter`, () => {
+    expect(checkPassword('P@SSW0RD!')).toEqual(false);
+  });


This test expects false for 'P@SSW0RD!' (password without small letters). The task requirements do not require at least one lowercase letter; they require at least one uppercase, one digit and one special character. A password missing lowercase but having an uppercase, digit and special should be valid. Update this test (either remove it or change the expected result to true) so it matches the specification.

mateacademy-ai-mentor · 2025-10-03T15:12:35Z

src/checkPassword.test.js

+  });
+
+  it(`should return 'false' when password without special character`, () => {
+    expect(checkPassword('Passw0rd')).toEqual(false);


There is no test for a password missing a digit. Add a test asserting false when the string has uppercase and special characters but no digits (example to add): expect(checkPassword('Password!')).toEqual(false); — this verifies the "at least 1 digit" requirement.

mateacademy-ai-mentor · 2025-10-03T15:12:35Z

src/checkPassword.test.js

+  });
+
+  it(`should return 'false' when the password is more than 16 characters`, () => {
+    expect(checkPassword('P@ssw0rd!P@ssw0rd!')).toEqual(false);


Add a test that verifies the upper length boundary is accepted (exactly 16 characters). Current tests check >16 is invalid but not that 16 is valid. Add an assertion like: expect(checkPassword('P@ssw0rd!P@ssw0r')).toEqual(true); (verify the string length is exactly 16 before using it).

mateacademy-ai-mentor · 2025-10-03T15:12:35Z

src/checkPassword.test.js


  it(`should return boolean`, () => {
-
+    expect(typeof checkPassword('12345678')).toEqual('boolean');


Add explicit tests for the examples from the task description so they are covered by the suite: checkPassword('Password1!') === true, checkPassword('qwerty') === false, and checkPassword('Str@ng') === false. Place these near other simple example tests so the specification examples are validated.

mateacademy-ai-mentor · 2025-10-03T15:12:35Z

src/checkPassword.test.js

+  });
+
+  it(`should return 'false' when password with spaces`, () => {
+    expect(checkPassword('P@s sw0rd!')).toEqual(false);


Add tests that verify non-Latin letters are rejected. The requirements state only Latin letters A–Z/a–z are allowed. For example: expect(checkPassword('Pässw0rd!')).toEqual(false); (accented ä) and expect(checkPassword('Рassword1!')).toEqual(false); (Cyrillic Р).

mateacademy-ai-mentor · 2025-10-03T15:12:36Z

src/checkPassword.test.js

+
+  it(`should return 'false' when password without special character`, () => {
+    expect(checkPassword('Passw0rd')).toEqual(false);
+  });


Add the requested edge-case checks: (1) a minimal valid password with exactly one of each required category (uppercase, digit, special) and total length >= 8, e.g. expect(checkPassword('A1!aaaaa')).toEqual(true); (verify composition/length); (2) multiple specials/digits still valid within limits; and (3) a password with uppercase and special but lacking a digit should be invalid (e.g. expect(checkPassword('Passw@rd')).toEqual(false);). These cover the combinations explicitly required by the description.

mateacademy-ai-mentor

Review Summary

Decision: REQUEST_CHANGES

Your tests are close, but two critical expectations contradict the spec and will cause correct implementations to fail.

Critical issue 1 (src/checkPassword.test.js): A test asserts that a password without lowercase letters (e.g., "P@SSW0RD!") is invalid. The spec does not require lowercase letters; it only requires at least one uppercase letter, one digit, one special character, and that letters are Latin. Update the test description to reflect that lowercase is not required and change the expectation to toEqual(true), or remove this test if you intended to require lowercase (and then update the spec accordingly).
Critical issue 2 (src/checkPassword.test.js): A test currently expects "Pässw0rd!" to be valid. The spec states passwords must accept only Latin letters A–Z/a–z; the accented ä is non‑Latin and should be rejected. Change the expectation to toEqual(false) and update the test description to clearly state that non‑Latin (accented) letters are invalid, aligning it with your Cyrillic example.

Minor suggestions: ensure test names describe the spec rule being validated and consider adding a positive example that explicitly shows lowercase is optional. Once these two tests are fixed, the suite will accurately enforce the requirements. Keep up the solid work on boundary and requirement coverage.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-10-03T15:35:06Z

src/checkPassword.test.js

+  it(`should return false when password has no lowercase letter (function requires both upper and lower)`, () => {
+    expect(checkPassword('P@SSW0RD!')).toEqual(false);
+  });


This test asserts the function requires lowercase letters and expects 'P@SSW0RD!' to be invalid. The task specification does NOT require lowercase letters — it only requires at least one uppercase letter, one digit and one special char (and only Latin letters overall). Update this test to match the spec. Options:

If you want to keep this check, change the expectation to toEqual(true) and update the test description to something like: should return true when password has no lowercase letter (lowercase is not required by spec).

Or remove/replace the test if you intended to require lowercase (but if so, update the task description accordingly).

Make sure the test description and assertion agree with the required behaviour in the description file.

mateacademy-ai-mentor · 2025-10-03T15:35:06Z

src/checkPassword.test.js

+  it(`should return true when password contains non-Latin letters like ä (function does not block them)`, () => {
+    expect(checkPassword('Pässw0rd!')).toEqual(true);
+  });


This test currently expects 'Pässw0rd!' to be valid. The description requires passwords to "accept only letters of the Latin alphabet Aa-Zz", so accented characters (like ä) are non‑Latin and should be rejected. Update this test to expect toEqual(false) and revise the test description to state that passwords containing non‑Latin (accented) letters must be invalid. You already have a Cyrillic example that expects false — make the accented example consistent with the spec.

mateacademy-ai-mentor

Review Summary

Educational approval to help you progress. Overall, your Jest suite is solid: it covers length boundaries, required character classes, and common examples. A few items need adjustment to align with the spec:

At src/checkPassword.test.js:39–41, you mark passwords without lowercase as invalid, but lowercase is optional per spec. Change the description to reflect “lowercase optional” and expect true for checkPassword('P@SSW0RD!').
At src/checkPassword.test.js:67–69, you accept accented letters, but only Latin A–Z/a–z are allowed. Update to expect false for checkPassword('Pässw0rd!') and clarify the description (non‑Latin letters invalid).
Add a negative test ensuring no letters is invalid (e.g., expect(checkPassword('1!2345678')).toEqual(false)) so digits/specials don’t substitute for required letters.

Minor: make test names explicitly state the rule being verified (e.g., “Latin letters only” and “lowercase optional”). Nice work covering boundary cases and special-character handling—these small fixes will make the suite fully compliant.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-10-04T13:19:33Z

src/checkPassword.test.js

+  it(`should return false when password has no lowercase letter`, () => {
+    expect(checkPassword('P@SSW0RD!')).toEqual(false);
+  });


Contradiction with spec — lowercase is optional.

The test at lines 39–41 currently asserts that a password with no lowercase letters ('P@SSW0RD!') should be false. The task specification requires at least one uppercase letter, one digit, and one special character, but it does NOT require lowercase letters. Update this test so the description reflects that lowercase is optional and the expectation is toEqual(true) (or remove the test if you intend to change the spec). Example change:

it('should allow passwords without lowercase letters (lowercase optional)', () => { expect(checkPassword('P@SSW0RD!')).toEqual(true); });

Leaving the test as-is will fail correct implementations that follow the spec.

mateacademy-ai-mentor · 2025-10-04T13:19:33Z

src/checkPassword.test.js

+  it(`should return true when password contains non-Latin (accented) letters like ä`, () => {
+    expect(checkPassword('Pässw0rd!')).toEqual(true);
+  });


Non‑Latin (accented) letters should be rejected.

The test at lines 67–69 currently expects checkPassword('Pässw0rd!') to be true. The spec states the password must accept only Latin letters A–Z/a–z, so accented characters like ä are non‑Latin and should be rejected. Change the expectation to toEqual(false) and update the description to indicate accented/non‑Latin letters are invalid. For example:

it('should return false when password contains accented/non‑Latin letters (e.g. ä)', () => { expect(checkPassword('Pässw0rd!')).toEqual(false); });

This aligns the test suite with the requirement about allowed letter ranges.

mateacademy-ai-mentor · 2025-10-04T13:19:33Z

src/checkPassword.test.js

+  it(`should return false when password has uppercase + special but no digit (Passw@rd)`, () => {
+    expect(checkPassword('Passw@rd')).toEqual(false);
+  });
 });


Missing test: ensure digits/specials don't substitute for letters.

The spec requires letters (and at least one uppercase). There is no test ensuring a password that contains only digits and special characters (no letters) is invalid. Add a negative test to confirm that the presence of digits/specials does not substitute for required letters. Add it near the other negative tests (for example, before the final cases). Example:

it('should return false when password has no letters (only digits/specials)', () => { expect(checkPassword('1!2345678')).toEqual(false); });

This test enforces that letters (A–Z/a–z) must be present and that digits/specials alone are insufficient.

mateacademy-ai-mentor · 2025-10-04T13:19:33Z

src/checkPassword.test.js

+  it(`should return false when password has no lowercase letter`, () => {
+    expect(checkPassword('P@SSW0RD!')).toEqual(false);
+  });

+  it(`should return false when password has no digit`, () => {
+    expect(checkPassword('Password!')).toEqual(false);
  });

-  it(`should return 'true' for the valid password with 8 characters`, () => {
+  it(`should return false when password has no special character`, () => {
+    expect(checkPassword('Passw0rd')).toEqual(false);
+  });

+  it(`should return false when password contains spaces`, () => {
+    expect(checkPassword('P@s sw0rd!')).toEqual(false);
  });

-  // write more tests here
+  it(`should return true for example password 'Password1!'`, () => {
+    expect(checkPassword('Password1!')).toEqual(true);
+  });
+
+  it(`should return false for example password 'qwerty'`, () => {
+    expect(checkPassword('qwerty')).toEqual(false);
+  });
+
+  it(`should return false for example password 'Str@ng'`, () => {
+    expect(checkPassword('Str@ng')).toEqual(false);
+  });
+
+  it(`should return true when password contains non-Latin (accented) letters like ä`, () => {
+    expect(checkPassword('Pässw0rd!')).toEqual(true);
+  });


Clarity suggestion — make test descriptions explicit.

After fixing the two contradictory tests above, consider updating a few test descriptions to be explicit about the rule they validate (e.g., "should return false when password contains non‑Latin letters", "should allow passwords without lowercase letters"). Clear descriptions reduce confusion about which rule each test enforces. The two problematic tests are at lines 39–41 and 67–69.

add tests for checkPassword function

a7901c4

mateacademy-ai-mentor suggested changes Oct 3, 2025

View reviewed changes

update tests

fa8b224

Alex-redman requested a review from mateacademy-ai-mentor October 3, 2025 15:30

mateacademy-ai-mentor suggested changes Oct 3, 2025

View reviewed changes

update tests again

a19692e

Alex-redman requested a review from mateacademy-ai-mentor October 4, 2025 13:14

mateacademy-ai-mentor approved these changes Oct 4, 2025

View reviewed changes


		it(`should return boolean`, () => {

		expect(typeof checkPassword('12345678')).toEqual('boolean');

Conversation

Alex-redman commented Oct 3, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants