Scotland | ITP JAN-2026 | Tuan Nguyen | Sprint 3 | 1-Implement-and-rewrite-test#1259
Scotland | ITP JAN-2026 | Tuan Nguyen | Sprint 3 | 1-Implement-and-rewrite-test#1259Jacknguyen4438 wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
…e first course work test
cjyuan
left a comment
There was a problem hiding this comment.
- Function implementation is correct.
Describing tests can be quite challenging. Feel free learn from AI how to keep the test descriptions concise.
| !cardRanks.includes(card.slice(0,-1)) || | ||
| !cardSuits.includes(card.slice(-1)) | ||
| ) { | ||
| throw new Error("Invalid card"); | ||
| } | ||
|
|
||
| switch (card.slice(0, -1)) { |
There was a problem hiding this comment.
Could consider storing the extracted rank and suit character in some variables first.
!cardRanks.includes(rank) || !cardSuits.includes(suit)
is probably more readable than
!cardRanks.includes(card.slice(0,-1)) || !cardSuits.includes(card.slice(-1))
| test(`should return "Invalid angle" when ( 180 < angle < 360)`, () => { | ||
| expect(getAngleType(-1)).toEqual("Invalid angle"); | ||
| expect(getAngleType(-10)).toEqual("Invalid angle"); | ||
| expect(getAngleType(361)).toEqual("Invalid angle"); |
There was a problem hiding this comment.
-
The test description is not quite correct.
-
Why not test both boundary cases?
| test(`should return false when denominator is infinity`, () => { | ||
| expect(isProperFraction(1, 23443243n)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
-
The test description does not quite match the test.
-
A large "bigint" type value is not the same as infinity. The closest thing to infinity in JavaScript is the predefined named constant
Infinity-- it is a value of type "number".
| test(`should return false when numerator is negative`, () => { | ||
| expect(isProperFraction(-1, 2)).toEqual(false); | ||
| }); | ||
| test(`should return false when denominator is negative`, () => { | ||
| expect(isProperFraction(1, -2)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
These test descriptions are not quite accurate.
For proper fractions, the condition is, "the absolute value of denominator is larger than the absolute value of the numerator". So we can create a category for proper fractions and describe it as:
should return true when abs(numerator) < abs(denominator).
| test("Should return 2 for '2♠'", () => { | ||
| expect(getCardValue("2♠")).toEqual(2); | ||
| }); | ||
|
|
||
| test("Should return 5 for '5♥'", () => { | ||
| expect(getCardValue("5♥")).toEqual(5); | ||
| }); | ||
|
|
||
| test("Should return 9 for '9♦'", () => { | ||
| expect(getCardValue("9♦")).toEqual(9); | ||
| }); | ||
|
|
||
| test("Should return 10 for '10♣'", () => { | ||
| expect(getCardValue("10♣")).toEqual(10); | ||
| }); |
There was a problem hiding this comment.
When preparing tests, we should ensure the tests cover all possible cases. If we specify a test for individual card, we will need about 53 tests to cover all possible cases. Instead, we could consider classifying all possible values into different categories, and then within each category we test some samples.
For example, one possible category for getCardValue() is, "should return the value of number cards (2-10)", and we can prepare the test as
test("should return the value of number cards (2-10)", () => {
expect(getCardValue("2♣︎")).toEqual(2);
expect(getCardValue("5♠")).toEqual(5);
expect(getCardValue("10♥")).toEqual(10);
// Note: We could also use a loop to check all values from 2 to 10.
});
Can you practice preparing tests in this fashion?
Learners, PR Template
Self checklist
Changelist
I have don't basic understand on how to correctly writing normal test and jest test frame work, I am waiting for this work to be review.
Questions
I have no question.