QA: Sprawdzenie poprawności hasła by JoAnna578 · Pull Request #157 · mate-academy/js_check_password

JoAnna578 · 2025-10-13T13:47:13Z

Funkcja checkPassword weryfikuje, czy hasło jest poprawne według zasad: długość 8–16 znaków, zawiera co najmniej jedną cyfrę, wielką literę i znak specjalny, oraz nie używa liter spoza alfabetu łacińskiego. Testy sprawdzają zarówno poprawne, jak i błędne hasła, w tym przypadki graniczne.

mateacademy-ai-mentor

Review Summary

I am requesting changes because several specification-critical tests are missing, which blocks verifying core requirements. Specifically, add explicit tests for the required examples: checkPassword('Password1!') === true, checkPassword('qwerty') === false, and checkPassword('Str@ng') === false (place near your existing positive/negative cases). Also add exact boundary tests: a 7-character invalid password (e.g., Abcde1! with 7 chars) and a 17-character invalid password that otherwise meets rules; these confirm the inclusive limits of 8–16. Complement your character-class coverage by adding a valid uppercase-only case (e.g., ABCDEF12! should be true). Expand non-Latin coverage with an accented Latin example (e.g., Pässword1! should be false if only ASCII is allowed). Finally, include a whitespace test (e.g., Abc def1!) and assert it’s rejected to make the policy explicit. Your current structure and many checks are solid—once these targeted tests are added, the suite will fully match the spec.

✨ 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-13T13:51:06Z

src/checkPassword.test.js


-describe(`Function 'checkPassword':`, () => {
-  const checkPassword = require('./checkPassword');
+const checkPassword = require('./checkPassword');


The import line is fine — require('./checkPassword') is correct and necessary for these tests. No change required here.

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return a boolean`, () => {
+    const result = checkPassword('Password1!');
+    expect(typeof result).toBe('boolean');
+  });


This test only asserts the return type for 'Password1!'. The task description explicitly lists checkPassword('Password1!') === true as a required example. Add a dedicated test asserting expect(checkPassword('Password1!')).toBe(true) so the example from the specification is covered exactly .

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return true for password with mix of allowed symbols`, () => {
+    const result = checkPassword('XyZ123@#');
+    expect(result).toBe(true);
+  });


Add explicit tests for the other two example strings from the description: checkPassword('qwerty') === false and checkPassword('Str@ng') === false. These examples are required by the spec and should be present as individual tests alongside the existing ones .

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return false for password shorter than 8 characters`, () => {
+    const result = checkPassword('Ab1!');
+    expect(result).toBe(false);


The short-password test uses 'Ab1!' (length 4). The spec requires an explicit check for a 7-character password (should be false). Add a test using exactly 7 characters so the inclusive lower boundary (8) is precisely verified (e.g., a 7-character string that otherwise contains required character classes where possible).

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return false for password longer than 16 characters`, () => {
+    const result = checkPassword('Abcdefghijklmnop1!');
+    expect(result).toBe(false);
+  });


The long-password test uses 'Abcdefghijklmnop1!', which is 18 characters. The spec requires an explicit check for a 17-character password (should be false) and that exactly 16 characters is accepted. You already test a 16-char valid case, but please include a 17-character invalid test specifically to validate the inclusive upper bound of 16.

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return false for password without uppercase letter`, () => {
+    const result = checkPassword('abcdef1!');
+    expect(result).toBe(false);
+  });


You test absence of uppercase (good). Complement this with a positive test that a password composed of only uppercase letters (plus at least one digit and one special char and valid length) is accepted — the spec does not require a lowercase letter. Example: expect(checkPassword('ABCDEF12!')).toBe(true) — add this to ensure uppercase-only but otherwise valid passwords pass.

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return false for password with non-Latin characters`, () => {
+    const result = checkPassword('Пароль1!');
+    expect(result).toBe(false);


You correctly test a Cyrillic password which should be rejected. Also add a test with accented Latin letters (e.g., 'Pässword1!' or 'Passwörd1!') to ensure accented characters are rejected if the implementation must accept only ASCII Latin letters (A–Z, a–z). This expands non-Latin coverage beyond Cyrillic as required by the spec.

mateacademy-ai-mentor · 2025-10-13T13:51:06Z

src/checkPassword.test.js

+  it(`should return false for password without special character`, () => {
+    const result = checkPassword('Abcdefg1');
+    expect(result).toBe(false);


The suite does not clarify whether whitespace is allowed. The spec implies special characters are allowed but whitespace is usually not intended. Add a test containing a space (e.g., 'Abcdef1 !' or 'Abc def1!') and assert the intended behavior (likely false). This makes the policy explicit and avoids ambiguity.

mateacademy-ai-mentor

Review Summary

I’m requesting changes because one required boundary case is not actually tested, which leaves a core requirement unverified.

Critical blocker: In src/checkPassword.test.js, the test labeled “should return false for 17-character password” uses Abcdefghijklmnop1!, which is 18 characters, not 17. This means the upper-length boundary (rejecting 17 characters) is not being verified as required. Please replace the string with an exact 17-character example that still includes at least one uppercase, one digit, and one special character (e.g., Abcdefghijklmno1! which is 17 chars) and assert false.

Minor (non-blocking) improvements:

In src/checkPassword.js, consider RegExp.prototype.test for boolean checks and add a guard if (typeof password !== 'string') return false; to avoid runtime errors.
Optionally refine the ASCII regex from [\x20-\x7E] to [\x21-\x7E] to make the no-space intent explicit.

Once the 17-character test is corrected, the suite will fully validate the required boundaries. Great job on the comprehensive rules and ASCII checks overall!

✨ 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-13T14:02:05Z

src/checkPassword.js

-  // eslint-disable-next-line
-  if (password.match(validPasswordRegex) && password.match(cyrillicValidation)) {
+  // regex sprawdzający tylko znaki ASCII
+  const asciiValidation = /^[\x20-\x7E]*$/;


The asciiValidation range currently allows the space character because it uses [\x20-\x7E] (0x20 is space). You already prevent whitespace with the validPasswordRegex (the (?!.*\s) part), so this is not causing a failing test — but it is slightly redundant. If you want the ASCII check to explicitly disallow spaces as well (making the intent clearer), consider tightening the range to start at \x21 so printable-space is excluded, e.g. const asciiValidation = /^[\x21-\x7E]*$/;. This makes the two checks less overlapping and more explicit about the allowed character set.

mateacademy-ai-mentor · 2025-10-13T14:02:05Z

src/checkPassword.js

+  // regex sprawdzający tylko znaki ASCII
+  const asciiValidation = /^[\x20-\x7E]*$/;
+
+  if (password.match(validPasswordRegex) && password.match(asciiValidation)) {


Using String.prototype.match in the if condition works, but it returns an array (or null) which you then coerce to a boolean. For a boolean test it is clearer and slightly more efficient to use RegExp.prototype.test, e.g. if (validPasswordRegex.test(password) && asciiValidation.test(password)) { ... }. Also, if there's any chance password could be non-string, calling .match/.test may throw — you could explicitly ensure it is a string first (e.g. if (typeof password !== 'string') return false;) to avoid runtime errors in unexpected situations.

mateacademy-ai-mentor · 2025-10-13T14:02:05Z

src/checkPassword.test.js

+  it(`should return false for 17-character password`, () => {
+    expect(checkPassword('Abcdefghijklmnop1!')).toBe(false);


The test labeled should return false for 17-character password currently uses the string Abcdefghijklmnop1!. Count the characters: abcdefghijklmnop is 16 letters, plus 1 and ! makes 18 total — so this test does not actually assert a 17-character boundary. The task requires rejecting a 17-character password that otherwise meets the rules. Please replace the test string with a true 17-character example that still contains at least one uppercase letter, at least one digit and one special character. For example:

// 15 letters + digit + special = 17 characters (contains uppercase A) expect(checkPassword('Abcdefghijklmno1!')).toBe(false);

After this change the test will correctly verify the 17-character boundary as required by the spec (see description).

mateacademy-ai-mentor

Review Summary

Here is my approval: your solution meets the requirements with clear, readable code and thorough tests. The password validator correctly enforces length (8–16), at least one uppercase, one digit, one special character, rejects whitespace, and limits characters to printable ASCII; the tests cover boundary cases and representative examples well.

Minor improvement suggestions for learning:

In src/checkPassword.js, consider using /^[\x21-\x7E]+$/ instead of * to make the “at least one printable ASCII char” intent explicit (non-blocking).
In src/checkPassword.test.js, add a couple of non-string input cases (e.g., null, 123) to confirm the type guard behavior.

Excellent work tightening the 17-character test to a true 17-char string and keeping the regexes focused. Nicely done—your solution is approved!

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

Rate AI review example ↩

mateacademy-ai-mentor suggested changes Oct 13, 2025

View reviewed changes

JoAnna578 added 2 commits October 13, 2025 15:53

QA: Walidacja hasła

f34aa28

QA: Walidacja hasła – poprawne ASCII i wymagania znakowe

39f96a5

JoAnna578 requested a review from mateacademy-ai-mentor October 13, 2025 13:57

mateacademy-ai-mentor suggested changes Oct 13, 2025

View reviewed changes

JoAnna578 added 2 commits October 13, 2025 16:05

QA: Walidacja hasła – ASCII, długość i klasy znaków

79f0b09

QA: Walidacja hasła – ASCII, długość i klasy znaków 2

05670aa

JoAnna578 requested a review from mateacademy-ai-mentor October 13, 2025 14:07

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

View reviewed changes

		it(`should return false for 17-character password`, () => {
		expect(checkPassword('Abcdefghijklmnop1!')).toBe(false);

Conversation

JoAnna578 commented Oct 13, 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!

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!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants