Cape Town | 25-ITP-MAY | Asanda Dunn | Sprint 2 | Data Groups - #1447
asandagwala wants to merge 8 commits into
Conversation
asandagwala
added
📅 Sprint 2
hackertainment
added
Review in progress
|
The programs are generally good. Just test cases do not having enough coverage currently. Try to think of different scenarios when designing test cases. Keep it up. :-) |
hackertainment
added
Reviewed
asandagwala
changed the title
asandagwala
added
the
Needs Review
asandagwala
changed the title
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Did you run the tests before submitting this? I still see some tests marked as "todo"
In addition I have one comment about a task.
Almost there!
| }; | ||
|
|
||
| // console.log(`${address[2]}`) | ||
| console.log(`My house number is ${address[0]}`); |
There was a problem hiding this comment.
This tells me the house number is undefined? Is that right?
LonMcGregor
removed
the
Needs Review
| // console.log(`${address[2]}`) | ||
| console.log(`My house number is ${address[0]}`); |
There was a problem hiding this comment.
address is an object but not array. so it cannot use indexing to get an element. please try again. thank you.
| test("contains returns false when passed an array", () => { | ||
| expect(contains([], "a")).toBe(false); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
There should be more test cases within this test suite. Can you think of them and add more? Thank you.
There was a problem hiding this comment.
There should be more test cases such as empty array or array with one pair of country-currency code etc. Please add them to increase the testing coverage. thank you.
| queryParams[key] = value; | ||
| // Ignore empty pairs | ||
| if (pair === "") { | ||
| continue; |
There was a problem hiding this comment.
please use else for the rest of loop rather than continue keyword.
| // Given an invalid input like a string | ||
| // When passed to tally | ||
| // Then it should throw an error | ||
| test("tally throws an error when given a string", () => { | ||
| expect(() => tally("hello")).toThrow(); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
Besides string, there are other invalid inputs which would be good to add to the test suite as well. Thank you.
3 participants
TASK ID: CYF-1083
Self checklist
This PR contains the work done on Sprint 2 of the Data Groups module