-
-
Notifications
You must be signed in to change notification settings - Fork 337
Glasgow | 25-ITP-Sep | Fares Bakhet | Sprint 3 | coursework/sprint-3-implement-and-rewrite #755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
8738ad1
ed40750
e6d384c
a2b2e06
11efcb1
d440aaf
07685f8
283fe06
7139e24
29b1e29
4c6b2b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,22 @@ | ||
| // This statement loads the isProperFraction function you wrote in the implement directory. | ||
| // We will use the same function, but write tests for it using Jest in this file. | ||
| const isProperFraction = require("../implement/2-is-proper-fraction"); | ||
| const isProperFraction = require("../implement/2-is-proper-fraction"); // The path | ||
|
|
||
| test("should return true for a proper fraction", () => { | ||
| expect(isProperFraction(2, 3)).toEqual(true); | ||
| }); | ||
|
|
||
| // Case 2: Identify Improper Fractions: | ||
| test("should return false for an improper fraction", () => { | ||
| expect(isProperFraction(5, 4)).toEqual(false); | ||
| }); | ||
|
|
||
| // Case 3: Identify Negative Fractions: | ||
| test("should return true for a negative fraction", () => { | ||
| expect(isProperFraction(-1, 2)).toEqual(true); | ||
| }); | ||
|
|
||
| // Case 4: Identify Equal Numerator and Denominator: | ||
| test("should return false for equal numerator and denominator", () => { | ||
| expect(isProperFraction(3, 3)).toEqual(false); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,34 @@ test("should return 11 for Ace of Spades", () => { | |
| }); | ||
|
|
||
| // Case 2: Handle Number Cards (2-10): | ||
| test("should return 5 for Five of Hearts", () => { | ||
| const fiveofHearts = getCardValue("5♥"); | ||
| expect(fiveofHearts).toEqual(5); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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.
});
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cjyuan, Do I need to redo my tests again in a better way? Or is it just for future knowledge?😅😅
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not practice it now? |
||
| // Case 3: Handle Face Cards (J, Q, K): | ||
| test("should return 10 for Jack of Diamonds", () => { | ||
| const jackofDiamonds = getCardValue("J♦"); | ||
| expect(jackofDiamonds).toEqual(10); | ||
| }); | ||
| test("should return 10 for Queen of Clubs", () => { | ||
| const queenofClubs = getCardValue("Q♣"); | ||
| expect(queenofClubs).toEqual(10); | ||
| }); | ||
| test("should return 10 for King of Hearts", () => { | ||
| const kingofHearts = getCardValue("K♥"); | ||
| expect(kingofHearts).toEqual(10); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could generalise these tests into one test to "should return 10 for face cards (J, Q, K)" and check all three ranks J, Q, K). In addition, the function is not expected to check t the suit character. Indicating the suit character in the test description may send the developer the wrong message.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cjyuan, I have generalised the test. Could you explain what you mean by check t?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a typo. Should be just "check the ..." |
||
| // Case 4: Handle Ace (A): | ||
| test("should return 11 for Ace of Spades", () => { | ||
| const aceofSpades = getCardValue("A♠"); | ||
| expect(aceofSpades).toEqual(11); | ||
| }); | ||
| // Case 5: Handle Invalid Cards: | ||
| test("should return Invalid card rank for an invalid card rank", () => { | ||
| const invalidCard = getCardValue("1♣"); | ||
| expect(invalidCard).toEqual("Invalid card rank"); | ||
| }); | ||
| test("should return Invalid card rank for another invalid card rank", () => { | ||
| const anotherInvalidCard = getCardValue("B♦"); | ||
| expect(anotherInvalidCard).toEqual("Invalid card rank"); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking
angle > 90is optional because previous if-statements can guaranteeangleis always more than 90.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cjyuan, Thanks for that. I have removed the unnecessary part.