London | 25-ITP-September |Sophia Mohamed |Sprint 3 | 2 practice tdd#842
London | 25-ITP-September |Sophia Mohamed |Sprint 3 | 2 practice tdd#842saff-coder wants to merge 12 commits intoCodeYourFuture:mainfrom
Conversation
London | 25-ITP-Sept | Sophia Mohamed | Sprint 1 | coursework/sprint-1
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Sprint part (Coursework/sprint 3) doesn't match expected format (example: 'Sprint 2', without quotes) 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). |
saff-coder
added
the
Needs Review
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Sprint part (Coursework/sprint 3) doesn't match expected format (example: 'Sprint 2', without quotes) 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). |
saff-coder
changed the title
| test("should count multiple occurrences of a character", () => { | ||
| const str = "aaaaa"; | ||
| const char = "a"; | ||
| const count = countChar(str, char); | ||
| const Char = "a"; | ||
| const count = countChar(str,Char); | ||
| expect(count).toEqual(5); | ||
| }); |
There was a problem hiding this comment.
Could also merge this test into the previous test (they both test "multiple occurrences").
| test("should return '1st' for 1", () => { | ||
| expect(getOrdinalNumber(1)).toEqual("1st"); | ||
| }); | ||
| test("should return 'th' for 11, 12, 13", () => { | ||
| expect(getOrdinalNumber(11)).toBe("11th"); | ||
| expect(getOrdinalNumber(12)).toBe("12th"); | ||
| expect(getOrdinalNumber(13)).toBe("13th"); | ||
| }); | ||
| test("should return correct ordinal for numbers > 20", () => { | ||
| expect(getOrdinalNumber(21)).toBe("21st"); | ||
| expect(getOrdinalNumber(22)).toBe("22nd"); | ||
| expect(getOrdinalNumber(23)).toBe("23rd"); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
To ensure thorough testing, we need broad scenarios that cover all possible cases.
-
Listing individual values can quickly lead to an unmanageable number of test cases. You can consider generalise the first test to cover all numbers ending in 1 (but not in 11).
-
Creating a category for "11, 12, 13" is good. You can also generalise the test to cover all numbers ending in 11, 12, 13.
-
"All numbers > 20" as one category might be too broad.
There was a problem hiding this comment.
More testing scenarios were added
cjyuan
added
Reviewed
saff-coder
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Well done.
| //test("should count multiple occurrences of a character", () => { | ||
| // const str = "aaaaa"; | ||
| //const Char = "a"; | ||
| //const count = countChar(str,Char); | ||
| // expect(count).toEqual(5); | ||
| //}); | ||
|
|
There was a problem hiding this comment.
To keep the code clean, why not delete all unused code?
cjyuan
added
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
I have completed all the required tasks and tests