-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
independence of test cases vs reproducibility #26
Comments
The original reason for using minhash seeding was to help people fix their assertions or functions. Imagine you have a failing test, you patch your code, now you want to see if the test fails on the orginal test case. You have two options: rerun the test or rerun repro on the original test run return value. The former will run on different data, because the code has changed hence the seeding. The latter will run on the old data (good) and the old assertion (bad). We need to decouple the two things. One option is to allow the user to specify the seeding to the test function. That requires noting down the original seed and entering it, goes beyond the goals of the test function and actually weakens its strength as a rigorous tester of code and invites all sort of abuse (imagine people starting to set the seed in all their tests, because it makes life easier). The other option is to allow repro to use a different assertion that the one that originally failed. If the patch was not applied to the assertion but is in a script or package function, then a re-run or repro will pick that up automatically (after reloading the script or package). The only problem is with the assertion. It doesn't look like the best user interaction where one has to copy the assertion somewhere else (it is normally near or inside the test call itself) but it is the best compromise. If assertions are named functions, it's easier
|
So this has been solved by adding an assertion argument to repro. The last lingering question is whether we should completely disable seeding in |
Scenario: apply change C to version V. V clears tests T. V+C doesn't. Because of the code changes, some tests have been re-seeded (forming set T+C), and they happen to include the failing ones. We can ask two questions
with a table:
To focus on the interesting cases (anti-diagonal above)
If V clears T + C but V + C doesn't clear T that should point to a problem with C. This may be tricky as some of the new tests may not be applicable to V, for instance when there's a new feature. If V + C clears T but V doesn't clear T + C, that should point to a problem in V that C doesn't fix but only the tests in T + C detect. The other two cases make my head spin, which may indicate that they are not possible, or that I just had a martini, but there is no evidence of the latter around here. |
The reproducibility feature may have some undesirable statistical properties. The idea is that to estimate the probability of bugs under the test case distribution, one needs independence of the code from the test cases. In statistical parlance, to estimate generalization performance (bug freeness) of a model (function) one needs the model to be picked before the test set is chosen or without access to it. If we debug and modify a function based on pseudo-random test cases we may end up with an implementation that's tailored for the test cases. That has to deal with how the test function is seeding the RNG. Right now, it uses a minhash based on tokens of the deparsed-assertion. That is, the seed changes only for substantial changes in the assertion. Any change in the function to implement would not affect the seeding. An example is the following
What exactly the specs are for
ff
is not important, but its implementation is based on running the test, seeing where it fails and collecting the generated values. One would be tempted to conclude, assuming that the distribution implemented byrsize
is the correct distribution, that the probability of a bug is fairly low, seebinom.test(0, 100)
(in the sense that we have a confidence interval containing only low probabilities) but the conclusion would be completely wrong.The text was updated successfully, but these errors were encountered: