Missing tests for Yacht exercise (can lead to accepting incorrect solutions)

Existing code in test_yacht.zig for Yacht exercise only tests cases when dice in full-house and four-of-a-kind categories would have groups in certain positions when sorted. For example:

3,3,3,3,5 => four of a kind
3,3,3,5,5 => full house

but there were no tests for these cases

3,5,5,5,5 => four of a kind
3,3,5,5,5 => full house

I.e. the cases where group positions and lengths are switched around and a group with smaller number of elements goes first.

I saw several solutions which were hardcoding checks like

if (sorted[0] == sorted[3]) {
  // assume success in detecting four-of a kind
}

which is not correct, because it would fail in the 3,5,5,5,5 case while it shouldn’t.

I have created PR with fixes, but it was auto-closed with suggestion to post here, which is what I did now :slight_smile:

The patch itself can be viewed on Github.

I believe this is the same topic which was addressed here: Yacht doesn't cover all test cases

Indeed! If full test coverage is not the goal, then it’s settled.

The problem with full coverage is that it’s really really hard to actually cover every single case and it’s really easy to write code which works for all the given inputs but would fail for something which isn’t covered. The test cases are there to cover the requirements; a reasonable implementation which passes all the tests ought to and likely implements all the requirements … but doesn’t necessarily do so, as there’s always ways around that.

More generic testing could be done using Property Based Testing, I kept thinking about it seeing all the individual test fixtures.

This requires a bit different approach though, but it would surely catch more cases: you write a property test, a lot of random inputs are generated, property is tested.

And both approaches could be used and mixed: example-based tests and property-based tests.

Not sure if this is something which was also discussed anywhere.

Yeah. Property based testing is great in many ways and can be super helpful for better test coverage. However, getting that to work across 60+ languages would probably be fairly tricky :slightly_smiling_face: One of the wonderful things about the current Exercism setup is how easy it makes it to spin up new language tracks using the canonical data.

Yeah, I suspected it might not be too easy to integrate PBT right away.

Nice! As a user of Exercism I can see it indeed seems to work very good for many languages.

1 Like