Skip to content

Glasgow | 25-ITP-Sep | Alaa Tagi| Sprint 3 | coursework/sprint 3 stretch #765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Alaa-Tagi wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Glasgow | 25-ITP-Sep | Alaa Tagi| Sprint 3 | coursework/sprint 3 stretch #765

Alaa-Tagi wants to merge 14 commits into from
+85 −39

Conversation

@Alaa-Tagi
Copy link

@Alaa-Tagi Alaa-Tagi commented Oct 13, 2025 •
edited
Loading

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 successfully completed all the assigned tasks, which include:

1- Editing the FIND.JS and PASSWORD-VALIDATOR.JS files as required.

2- Developing Jest test cases for the PASSWORD-VALIDATOR module to ensure its functionality.

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
cjyuan
cjyuan reviewed Oct 25, 2025
View reviewed changes
Sprint-3/3-stretch/password-validator.test.js
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

@cjyuan cjyuan Oct 25, 2025

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
cjyuan
cjyuan reviewed Oct 30, 2025
View reviewed changes
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.

Sprint-3/3-stretch/password-validator.test.js
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 •
edited
Loading

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
@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 30, 2025
@Alaa-Tagi
Copy link
Author

Alaa-Tagi commented Oct 30, 2025

i did the changes that you have mentioned

cjyuan
cjyuan reviewed Oct 30, 2025
View reviewed changes
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.

The PR description is now very informative.

When you have time, do checkout the Markdown syntax. It is used all over GitHub for formatting text.

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

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

@cjyuan cjyuan Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Something about this test is still not quite right. Can you fix the issue?

@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
@github-actions
Copy link

github-actions bot commented Oct 30, 2025

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@cjyuan
Copy link
Contributor

cjyuan commented Oct 30, 2025

Looks good now.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Structuring-And-Testing-Data The name of the module. 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Alaa-Tagi @cjyuan