fgmacedo / python-statemachine

Python Finite State Machines made easy.
MIT License
854 stars 84 forks source link

Models instantiated in Djangos setUpTestData seem to skip conditions #424

Closed robinharms closed 5 months ago

robinharms commented 5 months ago

Description

The MachineMixin for Django seems to behave differently when models are used within django tests. Oddly enough, any objects created during setUpTestData seem to cause transitions to bypass checks like conditions.

What I Did

This is a simple django application. Create migrations, run tests. (I'm skipping some imports for stuff you probably know)

statemachines.py

class MySM(StateMachine):
    draft = State("Draft", initial=True, value="draft")
    published = State("Published", value="published")

    publish = draft.to(published, cond="let_me_be_visible")

models.py

class MyContext(models.Model, MachineMixin):
    state = models.CharField(max_length=30, blank=True, null=True)
    state_machine_name = "myapp.statemachines.MySM"
    state_machine_attr = "wf"
    let_me_be_visible = False

tests.py

from django.test import TestCase
from statemachine.exceptions import TransitionNotAllowed

from myapp.models import MyContext
from myapp.statemachines import MySM

class WFTests(TestCase):
    @classmethod
    def setUpTestData(cls):
        # Runs once
        cls.one = MyContext.objects.create()

    def setUp(self):
        # Runs before every test
        self.two = MyContext.objects.create()

    def test_one(self):
        # This test will fail
        with self.assertRaises(TransitionNotAllowed):
            self.one.wf.send("publish")

    def test_two(self):
        # This works as expected
        with self.assertRaises(TransitionNotAllowed):
            self.two.wf.send("publish")

    def test_three(self):
        # Managing this instance works if I call it like this instead.
        # So this test works
        wf = MySM(self.one)
        with self.assertRaises(TransitionNotAllowed):
            wf.send("publish")

The inner workings of the transitions are still a bit opaque to me, so sorry if there's something obvious I'm missing here.

Thanks for maintaining this great library!

fgmacedo commented 5 months ago

Hi @robinharms, how are you? I didn't get time to work on this. Do you still have this issue?

Given the cost of setup to reproduce the issue, could you please provide me with a toy repository that I could quickly jump into and debug the error?

robinharms commented 5 months ago

Sure no problem, I can't do it right away but I'll try to get it done tomorrow!

robinharms commented 5 months ago

Here you go! https://github.com/robinharms/pysm-django

Just ask if there's anything else I can do. Thanks a lot for this!

fgmacedo commented 5 months ago

Thanks, @robinharms! I was able to reproduce the issue locally.

Here's what I've found: The root cause of the issue lies in the setUpTestData() method. Every cls attr created in this method is wrapped in a TestData class, which performs a deepcopy() for each test, so this is designed to allow safe alteration of objects between tests on the same TestCase instance.

However, this conflicts with the creation strategy of state machines. The references of properties/methods used as actions/conditions are computed at the time of creation. So, when the state machine is cloned, the references still point to the original instance.

I'm currently exploring potential solutions to this problem, if any.

robinharms commented 5 months ago

Nice! Thanks a lot for this! đŸ˜„