Skip to content

Conversation

@Alaa-Tagi
Copy link

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

I have created a PR with completed tasks as required.

Questions

No questions.

@Alaa-Tagi Alaa-Tagi changed the title Glasgow | 25-ITP-Sep | Fares Bakhet | Sprint 3 | coursework/sprint 3 stretch Glasgow | 25-ITP-Sep | Alaa Tagi| Sprint 3 | coursework/sprint 3 stretch Oct 13, 2025
@Alaa-Tagi Alaa-Tagi added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Oct 13, 2025
Comment on lines 29 to 35
test("password has at least one uppercase letter", () => {

const password = "1234a";
const result = isValidPassword(password);
expect(result).toEqual(false);
}
);
Copy link
Contributor

Choose a reason for hiding this comment

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

The function can return false for multiple reasons.
To test a specific reason, choose an input that satisfies all other conditions except the one you're targeting. This way, if the function returns false, you can confidently attribute it to that specific condition.

For example, the function might return false for "1234a" not only because it lacks an uppercase letter, but also because it lacks a special symbol.
As a result, we can't be certain that the function correctly handles the case of passwords without uppercase letters, since multiple conditions are being violated simultaneously.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 25, 2025
@Alaa-Tagi Alaa-Tagi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 29, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Can you make the PR description more informative (focusing on telling people what you have changed?)

In your current description:

I have created a PR with completed tasks as required.

  • "have created a PR" -- This does not carry new info.
  • "completed tasks" -- would be helpful to specify what the tasks are.

Comment on lines 56 to 61
test("password has at least one symbol: (!, #, $, %, ., *, &)", () => {

const password = "abcdA1";
const password = "abcd12";
const result = isValidPassword(password);
expect(result).toEqual(false);
}
Copy link
Contributor

@cjyuan cjyuan Oct 30, 2025

Choose a reason for hiding this comment

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

There are two possible reasons the test could pass;
the new password does not contain any uppercase letter (in addition to not containing any symbol).

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Structuring-And-Testing-Data The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants