Skip to content

Solution#164

Open
Julia2063 wants to merge 3 commits intomate-academy:mainfrom
Julia2063:develop
Open

Solution#164
Julia2063 wants to merge 3 commits intomate-academy:mainfrom
Julia2063:develop

Conversation

@Julia2063
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because the current test suite does not yet fully cover the critical boundary requirements from the task description. You did a solid job covering many core rules (function existence, boolean return type, digit/special/uppercase checks, Cyrillic rejection, and >16 length rejection), but there are still some gaps that block full compliance.

The critical issues to fix are:

  • The test that claims to use an 8-character password currently uses Password1!, which is not 8 characters; either change the title or, better, change the string to an actually 8-character valid password such as Pass1!Ab so the minimum inclusive length is explicitly tested.
  • Add a positive test for an exactly 16-character valid password, in a separate it block, to confirm that the upper bound (16) is accepted.
  • Ensure the existing long-password test description is updated from "valid password with more than 16 characters" to something like "password with more than 16 characters" and assert false for this case (it should be rejected).
  • Add a negative test for a password shorter than 8 characters, using the example checkPassword('Str@ng') === false from the task description.

Once you add these boundary and example-based tests to src/checkPassword.test.js, your suite will align well with the specification and give much stronger confidence in the implementation. You’re very close; these focused adjustments will make your tests complete and robust.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

});

it(`should return 'true' for the valid password with 8 characters`, () => {
const isPasswordValid = checkPassword('Password1!');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test title claims it checks an 8-character password but the string used ('Password1!') is not 8 characters. Replace this with an exactly 8-character valid password (for example, 'Pass1!Ab') or change the test title to match the actual input so the lower-bound requirement is explicitly covered (requirement: at least 8 characters inclusive).

});

// write more tests here
it(`should return 'false' for the valid password

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says "valid password with more than 16 characters" which is confusing — a password longer than 16 should be invalid. Update the description to "password with more than 16 characters" and ensure this test asserts false for that too. Also add a separate test that asserts a password of exactly 16 characters is accepted (upper-bound inclusive).

// write more tests here
it(`should return 'false' for the valid password
with more than 16 characters`, () => {
const isPasswordValid = checkPassword('Password1fhrfherhfhwbefhve!');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided long password in this test should remain to verify rejection of >16 chars, but add a new positive test with an exactly 16-character valid password to confirm the upper bound is inclusive. Consider adding a dedicated it block rather than overloading this one.


it(`should return 'false' for the invalid password
without at least one digit`, () => {
const isPasswordValid = checkPassword('Password!');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no test for passwords shorter than 8 characters. Add a test asserting checkPassword('Str@ng') === false (example from the task description) to cover the minimum-length rejection requirement.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because two critical requirements from the task description are not yet properly covered by your tests, which means the specification is not fully met. First, the test that claims to validate an 8-character minimum-inclusive password currently uses 'Pass1!Ab!' (9 characters) in src/checkPassword.test.js; this needs to be updated to a genuinely 8-character valid password such as Pass1!Ab or the test title must be corrected so that the minimum-length boundary is explicitly and accurately tested. Second, the negative example checkPassword('Str@ng') === false from the task description is not present; you should either add a dedicated test asserting checkPassword('Str@ng') is false or replace your existing short-password negative test with this exact example to align with the specification. These two points are important because the task explicitly requires testing both the minimum length boundary and the provided example inputs, and omitting or mislabeling them weakens confidence that the implementation meets all rules. Aside from that, your coverage of other rules (digits, special characters, uppercase, Cyrillic rejection, and max length handling) looks solid. Once you fix the 8-character test and add the Str@ng example test, your suite will fully satisfy the core test requirements for this task.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

});

it(`should return 'true' for the valid password with 8 characters`, () => {
const isPasswordValid = checkPassword('Pass1!Ab!');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test title says this verifies an 8-character valid password, but the string used is 'Pass1!Ab!' which is 9 characters. Change the password to an actual 8-character valid example (for instance: Pass1!Ab) or update the test title to match the current string. The task requires explicitly testing the minimum inclusive length.


it(`should return 'false' for the invalid password
with less than 8 characters`, () => {
const isPasswordValid = checkPassword('Pass2!');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suite includes a short-password negative test using 'Pass2!', but the task description gives the example checkPassword('Str@ng') === false. Add a test that asserts checkPassword('Str@ng') is false (or replace the existing short-password test with this example) so the provided example is covered.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution as an educational override so you can keep progressing while you refine your tests.

You’ve done a solid job covering the core password rules: existence and boolean return of checkPassword, boundary lengths (8 and 16 characters), too-short and too-long passwords, missing digit/special/uppercase cases, and non-Latin (Cyrillic) rejection. The main remaining gaps are two examples that the task description explicitly requires as tests: a positive test for checkPassword('Password1!') === true and a negative letters-only test for checkPassword('qwerty') === false. To improve, add a new valid-password test for 'Password1!' near your other valid cases (right after the 16-character test) and a new invalid-password test for 'qwerty' near the short-password and letters-only tests; this will align your suite exactly with the specification’s examples. As an optional enhancement, you might later consider adding tests for whitespace or accented characters to make the “Latin-only” constraint even clearer.

Overall, your test coverage is strong and you’re very close to a fully spec-compliant suite—nice work, and keep building on this foundation.


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

Footnotes

  1. Rate AI review example

it(`should return 'true' for the valid password with 16 characters`, () => {
const isPasswordValid = checkPassword('Password1234567!');

expect(isPasswordValid).toBe(true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an explicit test for the example valid password from the task description: checkPassword('Password1!') === true. Place it with the other valid-password tests (for clarity, after the 16-character test). Example to add:

it("should return 'true' for the example valid password 'Password1!'", () => {
  const isPasswordValid = checkPassword('Password1!');
  expect(isPasswordValid).toBe(true);
});

This ensures the exact example given in the description is covered (see description examples) .

with less than 8 characters`, () => {
const isPasswordValid = checkPassword('Str@ng');

expect(isPasswordValid).toBe(false);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an explicit test for the letters-only negative example checkPassword('qwerty') === false. Place it with the other negative tests (for clarity, after the short-password test). Example to add:

it("should return 'false' for the invalid password with only letters ('qwerty')", () => {
  const isPasswordValid = checkPassword('qwerty');
  expect(isPasswordValid).toBe(false);
});

The task requires verifying that a password composed only of letters is invalid; the description lists qwerty as that example and it must be explicitly tested .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants