Skip to content

Comments

London | 26-ITP-May | Oussama Mouggal | Sprint 3 | coursework-practice-tdd#1029

Open
Oussama-Mouggal wants to merge 6 commits intoCodeYourFuture:mainfrom
Oussama-Mouggal:coursework/sprint-3-practice-tdd
Open

London | 26-ITP-May | Oussama Mouggal | Sprint 3 | coursework-practice-tdd#1029
Oussama-Mouggal wants to merge 6 commits intoCodeYourFuture:mainfrom
Oussama-Mouggal:coursework/sprint-3-practice-tdd

Conversation

@Oussama-Mouggal
Copy link

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

tested edge cases and rewirted functions to pass the npm test

@Oussama-Mouggal Oussama-Mouggal added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 23, 2026
@Edu-Vin Edu-Vin added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 23, 2026
Copy link

Choose a reason for hiding this comment

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

I believe there is still room for improvement for this file. You didn't test for the number ending with 11. Also, for the other test cases, there should be a test to verify that the except values will not have the appended string example. using 12 should return 12nd. If you still want to experiment, you can change your except valuese from those ending with 12, 13 to 22 and 33.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you @Edu-Vin for your valuable feedback and remarks. I added more cases to my test/code to test all posibilities.

test("should throw an error when count is negative", () => {
const str = "hello";
const count = -1;
expect(() => repeatStr(str, count)).toThrow();
Copy link

Choose a reason for hiding this comment

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

To take this further, why not also check if the expected message is returned?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your message. I added a comment which will return in case of an error message.

@Edu-Vin Edu-Vin added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 23, 2026
@Oussama-Mouggal Oussama-Mouggal added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants