Improve discrimination of database keys

[jobh]

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:

• Mixing in __qualname__ is cheap and simple. https://github.com/HypothesisWorks/hypothesis/issues/3446#issuecomment-1676272346

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).

• For the parametrized strategies, we might mix in the @given arguments (if we recognize the need - see https://github.com/HypothesisWorks/hypothesis/pull/3865#discussion_r1469613175).
• Plus, mix in __qualname__ because inner tests are often called... inner.
• This might not be enough for the helper functions though, as they typically also take an asserted predicate as input — for those, maybe a @database_key decorator where we can mix in the predicate as well? Do note, we already set database=None in at least some of these, which fixes the slowdown issue.

[Zac-HD]

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 @pytest.mark.parametrize()d tests, which could also be used here. Just assign whatever bytestring you like, and it'll be hashed into the database key along with everything else, e.g.: https://github.com/HypothesisWorks/hypothesis/blob/7b634834b1dfa00674d1b614a7bb857017bf8d7c/hypothesis-python/src/_hypothesis_pytestplugin.py#L297-L299

[jobh]

Hmm, I think we might add to the list

• Pick up _hypothesis_internal_add_digest from the currently executing test function instead of the function actually passed to given.

otherwise we'd duplicate keys for f.x.

@pytest.mark.parametrize("v", [1, 2])
def test_something(v):

    @given(st.integers(v))
    def inner(x):
        assert x >= v

    inner()

since the attribute is added to test_something and not inner. At that point, it might be more straightforward to just store the add_digest as a global/threadlocal rather than as an attribute.

[edit] ...actually, we could mix in currently executing nodeid regardless of parametrization... and call it a day. Too pytest specific?

[Zac-HD]

[edit] ...actually, we could mix in currently executing nodeid regardless of parametrization... and call it a day. Too pytest specific?

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.

[jobh]

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., _clean_source in get_digest and ignoring specifier in find()). This leads to collisions in find (naturally), and also f.x. test_attrs_inference_builds which is defined by the same name but slightly different strategy in two test modules.

Why is this? Is it beneficial to have key stability across changes in strategy?

[Zac-HD]

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 @settings() and @example() decorators doesn't affect the database key.