beer-song: Make canonical data more logical #661
Conversation
We were testing: - the first verse - (*) some random verse in the middle - each of the special case verses - (*) 4 consecutive verses - all verses The two that are marked with an asterisk are not very logical choices in terms of TDD. The first asterisk is about the generation of individual verses. The structure of the algorithm is that verses 99 through 3 all have the same implementation. Then verses 2, 1, and 0 are all unique. Instead of testing the first generic verse and some random generic verse, it makes more sense to test the outer bounds: the first (99) and the last (3). The second asterisk is about generating consecutive verses. When testing multiples, the natural progression isn't "4" then "all". The best test data is _the smallest number that is multiple_, which is two. Since that can be implemented with a simple concatenation of the upper and lower bounds (verse of N + verse of M), it's useful to add an extra case that pushes towards a loop. Again, we want the smallest number that suggests a loop, which is three consecutive verses, not four. Then, finally, we have the full integration test, which is all the verses.
|
PR #662 is set to merge into this branch... |
There was a problem hiding this comment.
I just checked it against the JSON schema and it is compliant.
In the future maybe we should reconsider the 3-level nesting, because it may not be needed here. Anyway, that reflects the original structure, so the file seems to have been correctly adapted to the new schema.
Just the exercise property needs to be changed.
| } | ||
| ] | ||
| } | ||
| "exercise": "beer-run", |
There was a problem hiding this comment.
I guess that the exercise's name should be beer-song, which is the slug.
There was a problem hiding this comment.
meh, not sure why I put that in there, I know it is beer-song. I can fix it now
kotp
force-pushed
the
canonical-beer-song
branch
from
6ffaf1a to
9f3d48a
Compare
|
What does is the "property" property supposed to contain? We have the section |
|
I'll check that now, @kytrinyx. |
|
Ops! I guess let that one slip when reviewing. Sorry! Just opened #694 to fix that.
The I agree that the name is a bit strange at first, because most people are used to tests in the form But it is easy to create tests that do not fit in that format: Also, there is the extreme case of property-based testing, which we don't have yet, AFAIK, but is the most general case. Putting in simple terms, we are always testing some property of the solution, and hence this name. Makes sense? |
That makes sense. Thanks for clarifying! |
We were testing:
The two that are marked with an asterisk are not very logical choices in
terms of TDD.
The first asterisk is about the generation of individual verses.
The structure of the algorithm is that verses 99 through 3 all have the
same implementation. Then verses 2, 1, and 0 are all unique.
Instead of testing the first generic verse and some random generic
verse, it makes more sense to test the outer bounds: the first (99) and
the last (3).
The second asterisk is about generating consecutive verses.
When testing multiples, the natural progression isn't "4" then "all".
The best test data is the smallest number that is multiple, which is
two. Since that can be implemented with a simple concatenation of the
upper and lower bounds (verse of N + verse of M), it's useful to add
an extra case that pushes towards a loop.
Again, we want the smallest number that suggests a loop, which is three
consecutive verses, not four.
Then, finally, we have the full integration test, which is all the verses.