GoogleChromeLabs/jsbi

Improve test coverage

mathiasbynens opened this issue · 7 comments

mathiasbynens commented

The current implementation has been tested quite well, but even more rigorous testing is always possible.

Should you find any bugs, please report them!

dubzzz commented

Why not using property based testing on such lib (in addition to regular tests)? It applies quite well for such kind of lib

dubzzz commented

I already worked on such tests adapted for bigint. You might be interested in https://runkit.com/dubzzz/5b9f6183f459f700125eee17 (it checks big-integer npm package on multiple properties thanks to fast-check framework)

dubzzz commented

@mathiasbynens I did write some basic property based tests, more: dubzzz@7c7f6d5

jakobkummerow commented

Thanks for these contributions, I appreciate the eagerness to help!

I must admit that I'm not a huge fan of this particular approach though. For example, testing that a & b === b & a and a | b === b | a is nice and well, but doesn't actually detect whether the operations are implemented correctly -- hypothetical example, if the implementation mixed up & and |, such a test would not detect it.

My understanding is that the big argument in favor of property-based testing is that it's a form of randomized testing, where an automated test generator potentially generates test cases that a human might not have thought of. The obvious difficulty of random-generated test cases is how to generate expected values, which is where the properties come into play: they replace specific expectations with general invariants. In case of BigInts, we have what I believe is an even better option: use another language's implementation to generate assumed-correct expectation values. That's what the existing generator does: it uses Python to compute c such that it can actually test that a & b === c, which is a stronger assertion, because it checks that & is actually producing correct results. I have a nearly finished patch to extend that script's abilities to generate faster-running correctness tests in addition to the performance-oriented (but also correctness-checking!) benchmarks it currently generates.

mathiasbynens commented

I agree with @jakobkummerow; this is already sufficiently handled by the test generator.

dubzzz commented

Hi @jakobkummerow,

Indeed, the option of testing against other languages is obviously a good thing in this precise example. I did not think of that possibility at the beginning.

Meanwhile you can still rely on "randomly" generated a and b to run against the two implementations and compare their respective outputs. So that you confirm they are consistent on a large range of possible inputs.

Thanks for your answers :)