syrusakbary / snapshottest

Snapshot Testing utils for Python 📸
MIT License
525 stars 102 forks source link

Django test database not flushed and/or transaction not rolled back unless specifying django.test.TestCase mixin #139

Closed mcabrams closed 3 years ago

mcabrams commented 3 years ago

I'm seeing issues of data either not being flushed after each snapshottest.TestCase unless I specifically include a mixin for django.test.TestCase. I think the issue appears when a snapshot fails (data persists into next test case).

Here's the code I originally had:

from django.test import TestCase
from snapshottest import TestCase as SnapshotTestCase

class FooTestCase(SnapshotTestCase):
    def test_foo(self):
        # Create some instances of models in the database
        # Have some assertions that fail

class BarTestCase(SnapshotTestCase):
    def test_bar(self):
        # Filter for instances created in `test_foo` and 
        # you will find them (or they just may break your test here)

Changing to this eliminated the issue for me:

from django.test import TestCase
from snapshottest import TestCase as SnapshotTestCase

class FooTestCase(TestCase, SnapshotTestCase):
    def test_foo(self):
        # Create some instances of models in the database
        # Have some assertions that fail

class BarTestCase(TestCase, SnapshotTestCase):
    def test_bar(self):
        # Filter for instances created in `test_foo` and you won't find them

Seeing from #1 and looking at the source, TestCase is clearly inherited from, so that should not be the issue. I'm assuming there is some logic in test cleanup that has been overriden? I'm not sure.

paulmelnikow commented 3 years ago

Hi! Which release of snapshottest are you using? Could you check if this is reproducible in both 0.5.1 and 0.6.0?

mcabrams commented 3 years ago

I was on 0.6.0 - checked 0.5.1 and I have the same issue

medmunds commented 3 years ago

You're using the non-Django version of snapshottest's TestCase, which doesn't include Django's code to reset the DB between tests. (The naming is super confusing here. But Django kind of started it.)

The fix should be:

# Change
from snapshottest import TestCase as SnapshotTestCase
# To:
from snapshottest.django import TestCase as SnapshotTestCase

Your mixin approach does [edit: almost] exactly the same thing as snapshottest.django.TestCase. [Edit: your code flips the superclass order, but that shouldn't matter here.]

mcabrams commented 3 years ago

Thanks very much @medmunds, sorry I missed that! Closing this issue.