Skip to content
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

stateful testing prints undefined variables #2139

Closed
jbweston opened this issue Oct 16, 2019 · 4 comments · Fixed by #2145
Closed

stateful testing prints undefined variables #2139

jbweston opened this issue Oct 16, 2019 · 4 comments · Fixed by #2145
Labels
bug something is clearly wrong here

Comments

@jbweston
Copy link
Contributor

jbweston commented Oct 16, 2019

The Problem

When stateful prints a falsifying example sometimes the example contains variables that are not defined. This means that one cannot simply copy/paste the example into a Python shell to reproduce it.

Example

import hypothesis.strategies as st
from hypothesis.stateful import (
    Bundle, RuleBasedStateMachine, rule, consumes, multiple
)

class Foo(RuleBasedStateMachine):

    bar = Bundle("bar")

    @rule(target=bar, l=st.lists(st.integers(), 2))
    def step_one(self, l):
        return multiple(*l)

    @rule(n=consumes(bar), m=consumes(bar))
    def step_two(self, n, m):
        assert n + m > 10

TestFoo = Foo.TestCase

Running the above test (with pytest) I get the following traceback:

_________________________________________________ TestFoo.runTest __________________________________________________

self = <hypothesis.stateful.Foo.TestCase testMethod=runTest>

    def runTest(self):
>       run_state_machine_as_test(state_machine_class)

.local/miniconda/lib/python3.7/site-packages/hypothesis/stateful.py:243: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.local/miniconda/lib/python3.7/site-packages/hypothesis/stateful.py:141: in run_state_machine_as_test
    run_state_machine(state_machine_factory)
.local/miniconda/lib/python3.7/site-packages/hypothesis/stateful.py:89: in run_state_machine
    @given(st.data())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = Foo({'bar': []}), n = 0, m = 0

    @stateful.rule(n=stateful.consumes(bar), m=stateful.consumes(bar))
    def step_two(self, n, m):
>       assert n + m > 10
E       AssertionError: assert (0 + 0) > 10

poc.py:14: AssertionError
---------------------------------------------------- Hypothesis ----------------------------------------------------
Falsifying example: run_state_machine(factory=Foo, data=data(...))
state = Foo()
v1 = state.step_one(l=[0, 0])
state.step_two(m=v2, n=v1)
state.teardown()

Note that the falsifying example at the bottom of the traceback refers to a variable v2, which was never defined.

I suspect that this has to do with the fact that rule_1 in the example returns stateful.multiple.

Extra Information

Python version: 3.7.3
Hypothesis version: 4.36.2

@jbweston
Copy link
Contributor Author

jbweston commented Oct 16, 2019

AFAICT the problem is that RuleBasedStateMachine._print_step does not know about rules that return MultipleResult (this line here), but then when executing a step new variable names are assigned to each of the results in the MultipleResult here

@jbweston
Copy link
Contributor Author

One solution would be to have several variables assigned if a rule returns MultipleResult, e.g.

v1, v2 = state.step_one()  # Returns MultipleResult

A disadvantage of this is that if a large number of results are returned then we get an unwieldy selection of variables.

An alternative could be to allow MultipleResult to be indexable, so that the following could be produced:

v1 = state.step_one()  # Returns MultipleResult
state.step_two(m=v1[1])

The disadvantage of this is that the implementation would be more complicated because the state machine would need to keep track of MultipleResults separately (so that it would know which variable name and index to use), whereas now they are treated on the same footing as single results.

@DRMacIver
Copy link
Member

AFAICT the problem is that RuleBasedStateMachine._print_step does not know about rules that return MultipleResult (this line here), but then when executing a step new variable names are assigned to each of the results in the MultipleResult here

Yup, that's definitely it. Thanks for the bug report!

Changing the definition of upcoming_name should be enough to fix this I think.

@Zac-HD Zac-HD added the bug something is clearly wrong here label Oct 16, 2019
@DRMacIver
Copy link
Member

DRMacIver commented Oct 16, 2019

The disadvantage of this is that the implementation would be more complicated because the state machine would need to keep track of MultipleResults separately (so that it would know which variable name and index to use), whereas now they are treated on the same footing as single results.

This isn't necessarily as much of an issue as you might be imagining. Internally variable names are just strings, so there's no difficulty in principle with naming one v1[1] - they don't have to be valid python identifiers.

For maximum readability at the cost of a slightly more fiddly implementation (we do that a lot...) we could even switch between the two. e.g. if three or fewer results are returned, unpack it, if more than that are returned use indexing.

ETA: We could also not worry about it and just unpack them all actually. Generally what will happen (I think) is that the multiple will be hand-written and thus not too long or generated and thus will tend to get smaller during shrinking so should have a feasible number of variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is clearly wrong here
Projects
None yet
3 participants