pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.5k stars 525 forks source link

Tests depend on order of execution #411

Closed thedrow closed 4 years ago

thedrow commented 4 years ago

I verified this by running the test suite with pytest-randomly.

We cannot currently include this plugin in CI because of https://github.com/pytest-dev/pytest-xdist/issues/472.

The following tests randomly fail when the execution order is not correct:

1)

_________________________ TestWithGraphTransitions.test_pickle _________________________

self = <tests.test_nesting.TestWithGraphTransitions testMethod=test_pickle>

    def test_pickle(self):
        import sys
        if sys.version_info < (3, 4):
            import dill as pickle
        else:
            import pickle

        states = ['A', 'B', {'name': 'C', 'children': ['1', '2', {'name': '3', 'children': ['a', 'b', 'c']}]},
                  'D', 'E', 'F']
        transitions = [
            {'trigger': 'walk', 'source': 'A', 'dest': 'B'},
            {'trigger': 'run', 'source': 'B', 'dest': 'C'},
            {'trigger': 'sprint', 'source': 'C', 'dest': 'D'}
        ]
        m = self.stuff.machine_cls(states=states, transitions=transitions, initial='A')
        m.walk()
        dump = pickle.dumps(m)
        self.assertIsNotNone(dump)
        m2 = pickle.loads(dump)
        self.assertEqual(m.state, m2.state)
        m2.run()
>       m2.to_C_3_a()

tests/test_nesting.py:397:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <transitions.extensions.factory.HierarchicalGraphMachine object at 0x7ffb92122490>
name = 'to_C_3_a'

    def __getattr__(self, name):
        # Machine.__dict__ does not contain double underscore variables.
        # Class variables will be mangled.
        if name.startswith('__'):
            raise AttributeError("'{}' does not exist on <Machine@{}>"
                                 .format(name, id(self)))

        # Could be a callback
        callback_type, target = self._identify_callback(name)

        if callback_type is not None:
            if callback_type in self.transition_cls.dynamic_methods:
                if target not in self.events:
                    raise AttributeError("event '{}' is not registered on <Machine@{}>"
                                         .format(target, id(self)))
                return partial(self.events[target].add_callback, callback_type)

            elif callback_type in self.state_cls.dynamic_methods:
                state = self.get_state(target)
                return partial(state.add_callback, callback_type[3:])

        # Nothing matched
>       raise AttributeError("'{}' does not exist on <Machine@{}>".format(name, id(self)))
E       AttributeError: 'to_C_3_a' does not exist on <Machine@140718464181392>

transitions/core.py:1171: AttributeError

2)

__________________________ TestTransitions.test_wrong_nesting __________________________

self = <tests.test_reuse.TestTransitions testMethod=test_wrong_nesting>

    def test_wrong_nesting(self):

        correct = ['A', {'name': 'B', 'children': self.stuff.machine}]
        wrong_type = ['A', {'name': 'B', 'children': self.stuff}]
        siblings = ['A', {'name': 'B', 'children': ['1', self.stuff.machine]}]
        collision = ['A', {'name': 'B', 'children': ['A', self.stuff.machine]}]

>       m = self.machine_cls(states=correct)

tests/test_reuse.py:144:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
transitions/extensions/nesting.py:342: in __init__
    _super(HierarchicalMachine, self).__init__(*args, **kwargs)
transitions/core.py:574: in __init__
    self.add_model(model)
transitions/extensions/nesting.py:374: in add_model
    _super(HierarchicalMachine, self).add_model(models, initial=initial)
transitions/core.py:595: in add_model
    self._add_model_to_state(state, mod)
transitions/extensions/nesting.py:689: in _add_model_to_state
    self._add_model_to_state(state, model)
transitions/extensions/nesting.py:681: in _add_model_to_state
    if hasattr(model, method) and inspect.ismethod(getattr(model, method)) and \
transitions/core.py:1167: in __getattr__
    state = self.get_state(target)
transitions/extensions/nesting.py:555: in get_state
    return self.get_state(state, hint)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <transitions.extensions.nesting.HierarchicalMachine object at 0x7f1d7b5f2d30>
state = ['A'], hint = ['B', 'A']

    def get_state(self, state, hint=None):
        """ Return the State instance with the passed name. """
        if isinstance(state, Enum):
            state = [state.name]
        elif isinstance(state, string_types):
            state = state.split(self.state_cls.separator)
        if not hint:
            state = copy.copy(state)
            hint = copy.copy(state)
        if len(state) > 1:
            child = state.pop(0)
            try:
                with self(child):
                    return self.get_state(state, hint)
            except KeyError:
                with self():
                    state = self
                    for elem in hint:
                        state = state.states[elem]
                return state
        elif state[0] not in self.states:
>           raise ValueError("State '%s' is not a registered state." % state)
E           ValueError: State '['A']' is not a registered state.

transitions/extensions/nesting.py:563: ValueError

3)

_________________________ TestTransitions.test_blueprint_reuse _________________________

self = <tests.test_reuse.TestTransitions testMethod=test_blueprint_reuse>

    def test_blueprint_reuse(self):
        State = self.state_cls
        states = ['1', '2', '3']
        transitions = [
            {'trigger': 'increase', 'source': '1', 'dest': '2'},
            {'trigger': 'increase', 'source': '2', 'dest': '3'},
            {'trigger': 'decrease', 'source': '3', 'dest': '2'},
            {'trigger': 'decrease', 'source': '1', 'dest': '1'},
            {'trigger': 'reset', 'source': '*', 'dest': '1'},
        ]

        counter = self.machine_cls(states=states, transitions=transitions, before_state_change='check',
                                   after_state_change='clear', initial='1')

        new_states = ['A', 'B', {'name': 'C', 'children': counter}]
        new_transitions = [
            {'trigger': 'forward', 'source': 'A', 'dest': 'B'},
            {'trigger': 'forward', 'source': 'B', 'dest': 'C%s1' % State.separator},
            {'trigger': 'backward', 'source': 'C', 'dest': 'B'},
            {'trigger': 'backward', 'source': 'B', 'dest': 'A'},
            {'trigger': 'calc', 'source': '*', 'dest': 'C'},
        ]

        walker = self.machine_cls(states=new_states, transitions=new_transitions, before_state_change='watch',
                                  after_state_change='look_back', initial='A')

        walker.watch = lambda: 'walk'
        walker.look_back = lambda: 'look_back'
        walker.check = lambda: 'check'
        walker.clear = lambda: 'clear'

        with self.assertRaises(MachineError):
            walker.increase()
        self.assertEqual(walker.state, 'A')
        walker.forward()
        walker.forward()
        self.assertEqual(walker.state, 'C%s1' % State.separator)
        walker.increase()
        self.assertEqual(walker.state, 'C%s2' % State.separator)
        walker.reset()
        self.assertEqual(walker.state, 'C%s1' % State.separator)
        walker.to_A()
        self.assertEqual(walker.state, 'A')
        walker.calc()
>       self.assertEqual(walker.state, 'C_1')
E       AssertionError: 'C.1' != 'C_1'
E       - C.1
E       ?  ^
E       + C_1
E       ?  ^

tests/test_reuse.py:73: AssertionError

These are just the tests I managed to fail in a somewhat reproducible way. Tests should never depend on order as they might incorrectly fail the build.

aleneum commented 4 years ago

Hi @thedrow,

I also discovered these side effects when pulling your xdist changes. I started refactoring the tests to issolate different separators yesterday. I will probably finish this tonight.

aleneum commented 4 years ago

solved and merged into master