London | 26-ITP-Jan | Carlos Abreu | Sprint 3 | Practice TDD Coursework#1150
London | 26-ITP-Jan | Carlos Abreu | Sprint 3 | Practice TDD Coursework#1150carlosyabreu wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
carlosyabreu
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Function implementation is solid, especially on validating the parameters.
Test cases are comprehensive! Well done!
I think some of the test descriptions could be made more informative.
| }); | ||
|
|
||
| // Additional test cases to ensure robustness | ||
| test("should handle empty string", () => { |
There was a problem hiding this comment.
The person implementing the function may not immediately understand what "handle" means. It would be clearer to describe the expected behavior directly in the test description, such as specifying what the function should return.
Can you check which test descriptions (in all three test.js files) can be made more informative?
| */ | ||
|
|
||
| function getOrdinalNumber(number) { | ||
| if (typeof number !== 'number' || !Number.isInteger(number) || number < 0) { |
There was a problem hiding this comment.
One of the check here is redundant.
| }); | ||
|
|
||
| // Case 5: All other numbers (should get "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?
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
PR for Sprint-3 of Practice TDD Coursework