Document (or fix) limitations of stateful testing

[DRMacIver]

Because stateful testing is currently built on find, it had a number of features supported by @given that do not work. These include:

• Coverage guided testing (this is a particularly glaring omission because stateful testing is where this would be most useful!)
• Statistics
• (Most) health checks
• Deadlines

Currently this isn't documented anywhere. We should document this at a minimum, though better yet would be rebuilding stateful testing and given to have more of a common mechanism.

[Zac-HD]

Related to #1213, #1190, #164.

[DRMacIver]

I'm unsure how I feel about #164 these days. I think the reality may be that nobody dislikes the stateful API as much as I do and everyone hates my attempts to improve it more. Agreed those tickets are related, but I think #1213 is the only one that's directly related.

[Zac-HD]

I find bundles to be endlessly confusing, and would just rip them out deprecate them in favor of instance variables and the data() strategy.

Other than that, the inconsistencies you note above, and a trivial sense that I would have named things differently, it's not bad! The structure of a class with methods makes sense, and while I would probably use a class decorator (ala Attrs) to transform it and put a test in the global namespace IIRC that's hard to do compatibly.

Uh, that's a "hate your tools" rant, but I actually do quite like stateful testing despite seeing the flaws. ❤️

[DRMacIver]

I find bundles to be endlessly confusing, and would just rip them out deprecate them in favor of instance variables and the data() strategy.

I think that's more evidence that they need better documentation, because instance variables and the data strategy don't come close to replacing their functionality!

[DRMacIver]

(But I agree that most places where you can replace a bundle with an instance variable, you probably should)

[lennax]

This is a minor limitation, but @rule and @pytest.mark.parametrize are not compatible (unlike @given). I didn't see this reported in a separate issue.

MWE:

import pytest

from hypothesis import stateful
from hypothesis.strategies import booleans


class PytestMachine(stateful.RuleBasedStateMachine):

    @stateful.rule(i=booleans())
    @pytest.mark.parametrize('hi', (1, 2, 3))
    def parametrize_after_rule(self, hi, i):
        pass


TestPytestMachine = PytestMachine.TestCase

Failure:

 test_compat.py::TestPytestMachine::runTest <- [...]/python3.6/site-packages/hypothesis/stateful.py  
[...]/python3.6/site-packages/hypothesis/stateful.py:192: in runTest
    run_state_machine_as_test(state_machine_class)
[...]/python3.6/site-packages/hypothesis/stateful.py:110: in run_state_machine_as_test
    breaker.run(state_machine_factory(), print_steps=True)
[...]/python3.6/site-packages/hypothesis/stateful.py:248: in run
    state_machine.execute_step(value)
[...]/python3.6/site-packages/hypothesis/stateful.py:589: in execute_step
    result = rule.function(self, **data)
E   TypeError: parametrize_after_rule() missing 1 required positional argument: 'hi'
---------------------------------- Hypothesis ----------------------------------
Step #1: parametrize_after_rule(i=False)

[Zac-HD]

Some notes on rule-based stateful API friendliness, prompted by #1370:

• It would be nice to have @invariant work (you actually need @invariant())
• Similarly, @rule has a terrible error if used without calling it (TypeError trying to iterate over the decorated method as targets), which is an obvious mistake if you try to use it @given(...) to supply inputs and @rule to mark the method as a rule. I'm not inclined to do runtime checking for mixing with @given but a better message for decorate-time misuse seems reasonable.

[Zac-HD]

We would also like to provide the parts required to implement stateful testing downstream as public API, for use by e.g. async libraries. See #1371 for a proof-of-concept of how this might work.

Technically speaking you can just inherit from GenericStateMachine and replace the run method, but in practice I'm not sure how well this would work, mostly because cu.many(...) is crucial to have nice looping behavior. On the one hand a public equivalent would be nice; on the other just shrinking magically would be even better!

[njsmith]

Regarding async stateful testing, @Zac-HD wrote over here, on @touilleMan's PR:

Async stateful tests pose some harder problems in general though, because the order of the steps is no longer the only factor in the ordering - we might need a way for Hypothesis to take charge of the scheduler. In Trio's case this is actually OK as it's mostly-fair and we already seize the global PRNG so the random portion is reproducible. However, adversarial scheduling of coroutines and fault injection (timeouts, cancellations, mock network issues, etc.) would be very powerful for testing - as well as demanding library- and domain-specific implementation and user-facing controls

I'm replying here so my comment doesn't get lost on a random closed PR :-).

There are a lot of sources of possible non-determinism in your average Trio program (or any other program that uses concurrency or I/O). Providing ways to control this for testing is really important, which is why Trio provides things like a deterministic clock and deterministic "network connections". I'd love to have Hypothesis strategies like "a network connection with random chunking and delays", or "scheduler that randomly reassigned priorities to catch race conditions". However, I think all this is mostly orthogonal to stateful testing? All of these things are useful/necessary with regular @given-based tests (or even old-school non-Hypothesis-based tests), right?

[Zac-HD]

I agree on all counts - including both usefulness and orthogonality to stateful testing - but the implementation and more detailed discussion of that really belongs on a Trio issue tracker.

(Personally I'd consider having an optional dependency on Hypothesis within trio.testing, or split it out into a new package hypothesis-trio that could provide all those strategies. Getting further off-topic though...)

[touilleMan]

@Zac-HD @njsmith so what would be the next step now ?

I'm thinking to divide my previous POC into two PRs:

• one for hypothesis containing only the GenericStateMachine._custom_runner hook (btw I think _custom_runner is not a good name so suggestions wanted here ^^)
• one for trio adding hypothesis stateful support in trio.testing

[Zac-HD]

I don't think we (collectively) have enough experience with this kind of testing to write a useful API on the first attempt, so I'm feeling very cautious about adding something that would have to be supported for a long time.

Instead, I'd rework your proof-of-concept into a standalone library hypothesis-trio, with a class TrioGenericStateMachine that overrides the .run method directly. This can also be our basis for e.g. Hypothesis-based mock network objects, scheduler experiments, etc.

I'll be basically unavailable for the next week, but keen to be involved once my schedule calms down a little.

[touilleMan]

@Zac-HD @njsmith I've created the hypothesis-trio project with my POC in it.

Currently there is a monkey patch of hypothesis.stateful.StateMachineRunner.run that kicks in when hypothesis_trio.stateful is importer, it's not something I'm happy about but the alternative was to provide a custom version of StateMachineRunner, StateMachineSearchStrategy, find_breaking_runner and run_state_machine_as_test given they all make reference of each others...

Anyway, the good news is from now on we have hypothesis stateful working with trio \o/