clock: add more test cases to canonical test data #210
Conversation
| "expected": true | ||
| }, | ||
| { | ||
| "description": "clocks with negative hours and minutes that wrap", |
There was a problem hiding this comment.
this doesn't appear to be negative
and this appears to affect the test case's correctness since this results in 18:07 vs 5:53
There was a problem hiding this comment.
Oops, both of those were supposed to be negative. It's harder to test these--I should have regenerated the Go tests using this; I'll go do that now.
|
I generated the Go test suite locally to test the changes this time. |
|
👍 from me, though you've thought about this exercise more than I have. That means that while I am able to reason about whether the tests added by this PR make sense, I may miss tests that are not added by this PR that maybe should be. Luckily that's not so terrible since they can just be added in another PR. I did have one thought: Is there a test for adding 0 to a clock, and should there be? |
|
There is no such test, and I think that it's a good case to have. |
In the Go track I've seen a lot of people who only account for a clock overflowing once in either direction. As the README describes the problem, any overflow should be corrected, even if it accounts for several days. The second problem that I've seen a lot is people who adjust in the display method, which means that the clocks would fail the equality check. We weren't verifying equality against overflow, however, so we didn't catch this. I also removed two redundant test cases.
|
I've added the missing test. I regenerated the Go test suite and verified that it generates a passing set of tests. |
|
👍 , made a quick clock implementation and tested it against the json file, seems good |
|
Ok, thanks for being a second pair of eyes on this! |
Add hello world test generator
In the Go track I've seen a lot of people who only account for a clock
overflowing once in either direction. As the README describes the
problem, any overflow should be corrected, even if it accounts for
several days.
The second problem that I've seen a lot is people who adjust in the
display method, which means that the clocks would fail the equality
check. We weren't verifying equality against overflow, however, so we
didn't catch this.
I also removed two redundant test cases.