Repository navigation
Manchester | 26-ITP-Sept | Fatima Rouchi | Sprint 2 | Implement and rewrite tests - #1728
fatimarouchi wants to merge 11 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fatimarouchi
changed the title
fatimarouchi
added
the
Needs Review
cjyuan
added
Review in progress
|
|
||
| const value = Number(rank); | ||
|
|
||
| if (!isNaN(value) && value >= 2 && value <= 10) { | ||
| return value; | ||
| } |
There was a problem hiding this comment.
In JavaScript, strings that represent valid numeric literals in the language can be safely converted to equivalent numbers. For examples, "0x02", "2.1", or "0002".
Does your function return the value you expected from each of the following function calls?
getCardValue("0x02♠");
getCardValue("2.1♠");
getCardValue("0002♠");
| }); | ||
| // Case 6: Invalid angles | ||
|
|
||
| test('should return "Invalid angle" for invalid angles', () => { |
There was a problem hiding this comment.
An exact range of invalid angles could probably make the test description more informative.
| test("should return false for negative numerator and denominator", () => { | ||
| expect(isProperFraction(-1, -2)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
May I ask which authoritative source you used for the definition of a proper fraction?
In math
-
$\frac{-1}{2}$ and$\frac{1}{-2}$ equals$-\frac{1}{2}$ -
$\frac{-1}{-2}$ equals$\frac{1}{2}$
|
|
||
| test("Should return 10 when given a 10 card", () => { | ||
| 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 rank, we will need about 13 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?
| test("Should throw an error for invalid text", () => { | ||
| expect(() => getCardValue("invalid")).toThrow(); | ||
| }); | ||
|
|
||
| test("Should throw an error when suit is missing", () => { | ||
| expect(() => getCardValue("A")).toThrow(); | ||
| }); |
There was a problem hiding this comment.
When testing invalid inputs, focus on edge cases that the function might overlook. Inputs that closely resemble valid card values are especially useful. For example, "A" is a good test case because it contains a valid rank but is missing the suit character.
Can you think of other test cases that resemble valid card values but are still invalid?
An empty string ("") is also always worth testing for any parameter that accepts a string.
| test("classifies invalid negative angles", () => { | ||
| const invalid = getAngleType(-10); | ||
| assert.equal(invalid, "Invalid angle"); | ||
| }); | ||
| test("classifies angles greater than 360 as invalid", () => { |
There was a problem hiding this comment.
Note:
-
Negative angles means
angle < 0but0is also invalid. So you could consider specifyingwhen angle <= 0instead. -
The description "greater than 360" does not include
360.>= 360is "greater than or equal to 360".
cjyuan
added
Reviewed
Self checklist
##CYF-1059