-
-
Notifications
You must be signed in to change notification settings - Fork 409
London | 26-ITP-Sep | Bartosz Kawiak | Sprint 3 | implement-and-rewrite-tests #1624
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
base: main
Are you sure you want to change the base?
Changes from all commits
a97de34
9b3b9f6
c017a05
fff6736
ca55531
0681a40
71a49eb
1cefb86
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 |
|---|---|---|
|
|
@@ -11,10 +11,36 @@ test(`Should return 11 when given an ace card`, () => { | |
|
|
||
| // Suggestion: Group the remaining test data into these categories: | ||
| // Number Cards (2-10) | ||
| let suits = ["♠", "♥", "♣", "♦"]; | ||
| for (let number = 2; number <= 10; number++) { | ||
| for (let suit of suits) { | ||
| test(`should return card value as number`, () => { | ||
| expect(getCardValue(`${number}${suit}`)).toEqual(number); | ||
|
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. I appreciate the effort gone to test these thoroughly, but one thing to be careful of is if you make your tests so complicated that the tests themselves need testing. As a general rule, it's better to pick out specific cases that test the general input and the edge cases rather than needing to create loops and data structures to test every single possible input. Do you have nay thoughts on the approach you used here?
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. That makes sense. I wanted to check every possible number and suit to make sure they all worked correctly. I figured that using a loop would be the most efficient approach, but I can see how testing a few specific cases would make testing simpler. Thank you for the feedback. |
||
| }); | ||
| } | ||
| } | ||
| // Face Cards (J, Q, K) | ||
|
|
||
| let chars = ["J", "Q", "K"]; | ||
|
|
||
| for (let char of chars) { | ||
| for (let suit of suits) { | ||
| test(`should return card value as 10`, () => { | ||
| expect(getCardValue(`${char}${suit}`)).toEqual(10); | ||
| }); | ||
| } | ||
| } | ||
| // Invalid Cards | ||
|
|
||
| let invalidCards = ["22♥", "11K♠", -1, 3, "11^♠"]; | ||
| for (let card of invalidCards) { | ||
| test(`should return "Invalid card"`, () => { | ||
| expect(() => { | ||
| getCardValue(card); | ||
| }).toThrow(); | ||
| }); | ||
| } | ||
|
|
||
| // To learn how to test whether a function throws an error as expected in Jest, | ||
| // please refer to the Jest documentation: | ||
| // https://jestjs.io/docs/expect#tothrowerror | ||
|
|
||
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.
In this kind of maths, whenever the numerators value is lower than the denominator, regardless of ± sign, it is considered a true proper fraction. So -4/8 and 3/-5 are both valid proper fractions. Could you take another try at this?