Clean up use of example in tests

[jobh]

This patch cleans up usage of .example() in our tests, and un-ignores the associated warning by default. This is partly for performance, partly to see what happens if we dogfood our advice about not using it.

Except for testing of the `.example() code path itself, all usage falls into just two cases:

1. Verify that a strategy can produce examples, or that it fails with an appropriate exception,
2. Validating simple unvarying properties of examples (isinstance, is or == for constant strategies).

I added a new method in tests.common.debug for each of these use cases. They don't need to be new methods, both can be expressed by e.g. assert_all_examples, but:

• Method names express intent, which improves readability and saves need for commenting
• The methods allow us to centralize some decisions, e.g. how many examples to generate. If we before only checked a single .example() out of the 100 generated, maybe it's sufficient to now check all of the 15 generated. This saves 10%+ runtime in tests/cover.

(I know there are lower-hanging fruit for reducing runtime, but I enjoy saving cycles more)

[jobh]

I suspect this may be the cause of several data_too_large flakes I've seen recently.

With #3862, this would have stopped flaking, but still do needless replays.

If we ever invalidate database keys, we've already discussed adding __qualname__ for a slight improvement in discriminating tests. Maybe we should mix in the @given args as well, to cover this kind of wrapped tests?

[Zac-HD]

Hmm, yeah, I can see how that'd help. We could make it more precise by applying that logic only to tests which are indented but don't accept a self argument.

And we actually invalidate database keys pretty regularly for specific strategies, and I'm comfortable with a global-invalidation if our users get something out of it too. It's nice if we can bundle those into a few releases per year ofc, but I'd only suggest delaying such a release if we were actively working on another such change.

[jobh]

I don't have time to follow up on this now I'm afraid. I'll write up an issue to not forget it. Then, if no-one beats me to it, I might pick it up at a later date with a PR (to be merged at an opportune time). Sounds ok?

[Zac-HD]

Niiice, 10% savings is great!

I'm tempted to add a pytest hook in our conftest.py where if NonInteractiveExampleWarning is raised (b/c -Werror), we .add_note() a message suggesting use of 'one of the helper methods in tests.common.debug' - it might help discoverability for new and driveby contributors, which is the main downside IMO. Thoughts?

[Zac-HD]

Explicitly, I think this is an improvement on the status quo and trust you to merge it with or without my idea above. Thanks!

[jobh]

Niiice, 10% savings is great!

I'm tempted to add a pytest hook in our conftest.py where if NonInteractiveExampleWarning is raised (b/c -Werror), we .add_note() a message suggesting use of 'one of the helper methods in tests.common.debug' - it might help discoverability for new and driveby contributors, which is the main downside IMO. Thoughts?

I think that is a great idea! Will do. We don't need to actually check for -Werror do we, if we catch that exception it must be?

[Zac-HD]

Yep, no need to check warnings filter, just unconditionally annotate that exception.