-
Notifications
You must be signed in to change notification settings - Fork 586
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
Improve discrimination of database keys #3867
Comments
I agree that we should pick up any low-hanging improvements here, and this list looks pretty good to me. Specific to the last point, we have a magic attribute used to distinguish hypothesis/hypothesis-python/src/_hypothesis_pytestplugin.py Lines 297 to 299 in 7b63483
|
Hmm, I think we might add to the list
otherwise we'd duplicate keys for f.x.
since the attribute is added to [edit] ...actually, we could mix in currently executing |
Too specific - the problem is that if you execute a particular test function manually or via pytest, or a method with unittest or pytest, we want to have the same database key in either case. So mixing in the nodeid is fine when it's a parametrized test because there's a solid benefit and we're unlikely to execute it without pytest, but I'd prefer to avoid doing that unconditionally. |
Understood, thanks! I have just one more question: We don't use the strategy definition in our db key, and it looks intentional (e.g., Why is this? Is it beneficial to have key stability across changes in strategy? |
It's actually pretty hard to derive a stable-across-runs bytestring from an arbitrary strategy; for example it's easy to end up with a repr that includes some object's memory address, or less often something like the current time or date or machine name or OS. We strip the decorators from source code before hashing it so that adding or removing |
We don't (and can't) guarantee that our database keys are unique. If two test methods have identical keys, the consequences are limited: We replay arbitrary examples instead of previously-failing ones. This is a low-priority issue, but recorded here as a reminder.
While the downsides are limited, they do exist: a slight slowdown plus loss of the database benefits. Hence, we should try hard (within reason) to actually generate unique-yet-stable database keys.
Some ideas are:
__qualname__
is cheap and simple. Hypothesis Seed not reproducing "max allowable size" HealthCheck #3446 (comment)One common failure mode in our own tests is inner tests used in multiple contexts, either by parametrized strategies or inside a helper function (
tests.common.debug
).@given
arguments (if we recognize the need - see Clean up use of example in tests #3865 (comment)).__qualname__
because inner tests are often called...inner
.@database_key
decorator where we can mix in the predicate as well? Do note, we already setdatabase=None
in at least some of these, which fixes the slowdown issue.The text was updated successfully, but these errors were encountered: