Scotland | ITP JAN-2026 | Tuan Nguyen | Sprint 3 | 2- Practice-tdd#1258
Scotland | ITP JAN-2026 | Tuan Nguyen | Sprint 3 | 2- Practice-tdd#1258Jacknguyen4438 wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
| // Case 5: All other numbers | ||
| // When the number does not end with 1, 2, or 3, | ||
| // Then the function should append "th". | ||
| test("should append 'th' for all other numbers", () => { |
There was a problem hiding this comment.
When a test fails with the message "... all other numbers", it may be unclear what "other numbers" actually refers to.
Can you revise the test description to make it more informative?
There was a problem hiding this comment.
Thank you for the feed back I will fix it right away
There was a problem hiding this comment.
The test description still reads: "should append 'th' for all other numbers"
|
@cjyuan Hello thank you for the review, I see there are many part that need to be fix and improve. I will check all 5 part that you would like me to explain and fix each of them. When I finish I will mention you again to ask for a review. |
|
@cjyuan Hello, thank you for spending your time your time to do my PR, I am very grateful for you help. I have make change base on you suggestion and ready to be review |
| // Case 5: All other numbers | ||
| // When the number does not end with 1, 2, or 3, | ||
| // Then the function should append "th". | ||
| test("should append 'th' for all other numbers", () => { |
There was a problem hiding this comment.
The test description still reads: "should append 'th' for all other numbers"
|
@cjyuan Hello I have make change needed for review again. If I have make another error I need to be done please let me know. |
| test("should return error for invalid input", () => { | ||
| expect(() =>getOrdinalNumber(1.2)).toThrow("Invalid input"); |
There was a problem hiding this comment.
Note: "Return an error" is not the same as "throw an error".
| expect(() => getOrdinalNumber(1.2)).toThrow("Invalid input"); | ||
| expect(() => getOrdinalNumber(1.8)).toThrow("Invalid input"); | ||
| expect(() => getOrdinalNumber(2.5)).toThrow("Invalid input"); | ||
| expect(() => getOrdinalNumber(10.51)).toThrow("Invalid input"); | ||
| expect(() => getOrdinalNumber(12.49)).toThrow("Invalid input"); | ||
| expect(() => getOrdinalNumber(12.5)).toThrow("Invalid input"); |
There was a problem hiding this comment.
Why test so many decimal numbers with decimal point? If the objective is to show the function should treat any number with decimal points as invalid input, one or two samples should be enough.
Under the "invalid input" category, it would probably be more useful to show what other kinds of input are considered invalid. For examples,
- "300" -- a string that resembles an integer
- Infinity
- NaN
Learners, PR Template
Self checklist
Changelist
I have done practice and fully understand how to write a test for both normal JS and Jest test.
Questions
No Question.