Resetting DB sequences in Django

[jentyk]

Currently, AFAIK snapshottest.django.TestCase does not reset DB sequences between tests which django.test.TestCase does. It leads to the situation when snapshots differ depends on the order of test execution, i. e. IDs are different.

[thecylax]

Same situation here.
With SQLite, tests are OK, but changing to MySQL, it fails.

[paulmelnikow]

Would anyone like to create a PR to fix this?

[medmunds]

I'm unable to reproduce this. There is some potentially problematic subclassing going on in snapshottest.unittest and snapshottest.django, but I'm hesitant to change it without a reproducible problem.

@jentyk and @thecylax I realize it's been a couple of years, but do you happen to remember any details about what versions of Python, Django, and snapshottest you were using back in late 2018?

I've tried the test case below (by modifying the example django project) using Django 3.1, 2.2, and 1.11, with both the current snapshottest and v0.5.1 (though not in every combination), all on Python 3.7.

from lists.models import List
from snapshottest.django import TestCase

class ListTest(TestCase):
    def test_resets_db_1(self):
        self.assertEqual(0, List.objects.count())
        List.objects.create(name="test")
        self.assertEqual(1, List.objects.count())

    def test_resets_db_2(self):
        # This is the same test as above. We don't know what order
        # the tests will run, but if the DB isn't reset between them,
        # at least one will fail.
        self.assertEqual(0, List.objects.count())
        List.objects.create(name="test")
        self.assertEqual(1, List.objects.count())

Using mysql@5.7 via these settings:

DATABASES = {
    "default": {
        "ENGINE": "django.db.backends.mysql",
        'OPTIONS': {
            'read_default_file': '/home/me/.mysql/test.cnf',
        },
    }
}

[jentyk]

I'm unable to reproduce this. There is some potentially problematic subclassing going on in snapshottest.unittest and snapshottest.django, but I'm hesitant to change it without a reproducible problem.

@jentyk and @thecylax I realize it's been a couple of years, but do you happen to remember any details about what versions of Python, Django, and snapshottest you were using back in late 2018?

I've tried the test case below (by modifying the example django project) using Django 3.1, 2.2, and 1.11, with both the current snapshottest and v0.5.1 (though not in every combination), all on Python 3.7.

from lists.models import List
from snapshottest.django import TestCase

class ListTest(TestCase):
    def test_resets_db_1(self):
        self.assertEqual(0, List.objects.count())
        List.objects.create(name="test")
        self.assertEqual(1, List.objects.count())

    def test_resets_db_2(self):
        # This is the same test as above. We don't know what order
        # the tests will run, but if the DB isn't reset between them,
        # at least one will fail.
        self.assertEqual(0, List.objects.count())
        List.objects.create(name="test")
        self.assertEqual(1, List.objects.count())

Using mysql@5.7 via these settings:

DATABASES = {
    "default": {
        "ENGINE": "django.db.backends.mysql",
        'OPTIONS': {
            'read_default_file': '/home/me/.mysql/test.cnf',
        },
    }
}

Hmmm... your test seems not testing what it should test for. It seems testing if the DB if being torn down between tests, which it is.
My problem was completely different, if I remember correctly as it's been ages since then, the record ID sequences are not being reset between runs. In other words if I create one record in specific table in the first test to run it will get ID no 1 and in the next running test, if the class the test class is derived from is "django.test.TestCase", and I create one record in the same table its ID will be 1 again. However, if the test class is derived from "snapshottest.django.TestCase", the ID will be 2.
Why it is a problem? If we try to record snapshots of some records in DB the IDs will differ depends on the order the tests run, if you run all your collection of tests. If you run just single test or tests from one test class then it also can differ.
It is a nuisance as what we are talking about is snapshot testing, but you have to remove IDs from snapshots to make them comparable.

[medmunds]

[Summary: If your tests need isolated auto-increment field sequences, Django requires you use TransactionTestCase with reset_sequence = True. You can achieve that with the existing snapshottest, but it would be helpful for snapshottest to expose an augmented Django TransactionTestCase.]

Thanks for the clarification @jentyk. I somehow hadn't made the connection that "DB sequences" was referring to auto-increment fields. Now I have a reproducible case. But it fails with both Django's own TestCase and snapshottest's augmented version:

# This fails (with mysql) using *either* TestCase import
from lists.models import List
# from django.test import TestCase
from snapshottest.django import TestCase

class ListTest(TestCase):
    def test_resets_auto_fields_1(self):
        obj = List.objects.create(name="test")
        self.assertEqual(obj.id, 1)

    def test_resets_auto_fields_2(self):
        # This is the same test as above. We don't know what order
        # the tests will run, but if DB auto increment sequences
        # aren't reset reset between them, at least one will fail.
        obj = List.objects.create(name="test")
        self.assertEqual(obj.id, 1)

There are two things going on here:

1. In Django tests, you must specifically set reset_sequence = True to isolate auto-increment sequences between tests. (Django's default is False, because it can be expensive and specific ids don't matter for most tests. But as you point out, the ids do matter if you're snapshotting them.)

2. The reset_sequence option is only available on Django's TransactionTestCase, and can't be used with Django's optimized TestCase subclass (or snapshottest's augmented version of that). snapshottest doesn't directly export an augmented TransactionTestCase. But you can mix together your own:

# This works. (Also demonstrates snapshot test of serialized Django models.)
import snapshottest
from django.core.serializers import serialize
from django.test import TransactionTestCase
from lists.models import List

class ListTest(snapshottest.TestCase, TransactionTestCase):
    reset_sequences = True  # <-- critical for isolated ids on all DB engines

    def test_resets_auto_fields_1(self):
        obj = List.objects.create(name="test")
        self.assertEqual(obj.id, 1)
        self.assertMatchSnapshot(serialize('json', [obj]))

    def test_resets_auto_fields_2(self):
        obj = List.objects.create(name="test")
        self.assertEqual(obj.id, 1)
        self.assertMatchSnapshot(serialize('json', [obj]))

(Note that's mixing in the base snapshottest.TestCase, not snapshottest.django.TestCase.)

I think we should treat this as a feature request to expose a snapshottest-augmented TransactionTestCase. Given that it's likely to be used for snapshots, the snapshottest version should set reset_sequences True by default. And then we should document using it in cases where ids or other auto-increment fields will be snapshotted.

[paulmelnikow]

I think we should treat this as a feature request to expose a snapshottest-augmented TransactionTestCase.

I'm good with that 👍

Given that it's likely to be used for snapshots, the snapshottest version should set reset_sequences True by default.

Despite it being what most people would want, I think it is probably confusing to have a different behavior between our TransactionTestCase and Django's. I'd suggest we let users set reset_sequences themselves to avoid unexpected side effects.

We could also emit a warning in our TransactionTestCase when reset_sequences is unset, to really help them do the right thing.

[jentyk]

I think we should treat this as a feature request to expose a snapshottest-augmented TransactionTestCase.

Very good idea.

Despite it being what most people would want, I think it is probably confusing to have a different behavior between our >TransactionTestCase and Django's. I'd suggest we let users set reset_sequences themselves to avoid unexpected side >effects.

True

We could also emit a warning in our TransactionTestCase when reset_sequences is unset, to really help them do the right >thing.

Or just create another class with that variable True by default.
There is no need to mirror Django 100% but rather add some classes useful for snapshot testing.

@medmunds thanks for investigating 👍

[medmunds]

We could also emit a warning in our TransactionTestCase when reset_sequences is unset, to really help them do the right thing.

Or just create another class with that variable True by default.
There is no need to mirror Django 100% but rather add some classes useful for snapshot testing.

So I guess I'd argue that snapshottest.django.TransactionTestCase is that other class that is useful for snapshot testing, which is why it should have reset_sequences set to True. If you want a regular Django TransactionTestCase for some other, non-snapshot testing, you should probably just use django.test.TransactionTestCase instead.

However, having two different things called "TransactionTestCase" seems like it's just asking for confusion. (Or worse, three things called just "TestCase"—see #139.) Maybe snapshottest.django should instead expose SnapshotTestCase, SnapshotTransactionTestCase (with reset_sequences True), and SnapshotSimpleTestCase.

Users who need something different could, of course, always mix snapshottest.TestCase (er, SnapshotTestCaseMixin?) into any unittest.TestCase subclass to get whatever behavior they desire.

[paulmelnikow]

However, having two different things called "TransactionTestCase" seems like it's just asking for confusion. (Or worse, three things called just "TestCase"—see #139.) Maybe snapshottest.django should instead expose SnapshotTestCase, SnapshotTransactionTestCase (with reset_sequences True), and SnapshotSimpleTestCase.

To play devil's advocate here…

The layers we have, on the surface, seem to provide convenience, but at they also obscure what's going on. Another way we could go is the opposite. Instead of adding more convenience layers, we could deprecate these convenience classes, and require all Django users to use a mixin. That way there's one obvious way of doing it, and it's explicit.

Not totally sure the explicit way would be better, however I really don't like having ~8 classes which all have slightly different class hierarchies and behaviors and names which are very similar or identical to each other. It seems like a recipe for confusion.

[medmunds]

we could deprecate these convenience classes, and require all Django users to use a mixin

Oh, that sounds like it would feel much cleaner. Let's take it for a test drive in the (edit: hypothetical future!) docs...

Usage with Django

Add to your settings:

TEST_RUNNER = 'snapshottest.django.TestRunner'

To create your snapshottest:

from django.test import TestCase
from snapshottest import SnapshotTestCaseMixin

class APITestCase(SnapshotTestCaseMixin, TestCase):
    def test_api_me(self):
        """Testing the current user info API"""
        response = self.client.get('/api/me')
        self.assertMatchSnapshot(response.json())

To update the snapshots, run python manage.py test --snapshot-update.
Check the Django example.

SnapshotTestCaseMixin works with any unittest TestCase, including Django's TestCase, SimpleTestCase and TransactionTestCase. If your snapshots include Django model ids or other auto-increment fields, be sure to use TransactionTestCase with the reset_sequence option enabled to ensure reproducible ids:

from django.test import TransactionTestCase
from snapshottest import SnapshotTestCaseMixin

class APITestCase(SnapshotTestCaseMixin, TransactionTestCase):
    reset_sequence = True  # reset auto-increment fields for each test

    def test_api_posts(self):
        # Create a post:
        response = self.client.post('/api/posts', {"title": "test"}, content_type="application/json")
        self.assertMatchSnapshot(response.json())
        # Retrieve all posts:
        response = self.client.get('/api/posts')
        self.assertMatchSnapshot(response.json())

I like it!

(I also updated the example to be a little more Djangotic.)

[paulmelnikow]

I like it too.

SnapshotTestCaseMixin is a little long. I wonder if it could be SnapshotMixin?

[medmunds]

SnapshotTestCaseMixin is a little long. I wonder if it could be SnapshotMixin?

It could, but then I think we'd want to export it from snapshottest.unittest, not the package root, to avoid confusion with mixins for other test frameworks. (The words "TestCase" seem to be strongly associated with unittest.)

I'd probably also want to keep the word "Test" to distinguish from a "TestRunner" mixin. (We already have a TestRunnerMixin in django, and will have one soon in unittest.)

So: from snapshottest.unittest import SnapshotTestMixin?

[paulmelnikow]

Although, aren't mixins basically a pattern for unittest.TestCase? Pytest in particular does not use mixins. I haven't used nose in a little bit, but my recollection was that nose tests were still unittest.TestCase subclasses.

To be honest I'd rather have a flatter import structure than a nested one; if it's possible that all the things developers need can live in one namespace, it's simpler to talk about them.

I'd probably also want to keep the word "Test" to distinguish from a "TestRunner" mixin. (We already have a TestRunnerMixin in django, and will have one soon in unittest.)

Hmm, yes, this is a good point!

[jentyk]

However, having two different things called "TransactionTestCase" seems like it's just asking for confusion. (Or worse, three things called just "TestCase"—see #139.) Maybe snapshottest.django should instead expose SnapshotTestCase, SnapshotTransactionTestCase (with reset_sequences True), and SnapshotSimpleTestCase.

I do not agree, I think this is absolutely fine. We have to take the whole namespace under consideration and then obviously it differs. "django.test.TestCase" differs from "snapshottest.django.TestCase" even if the last part of the name is the same. There is no point of inventing weird and cryptic names just to make them differ from names of dozens of other classes which have the same name but sits in different packages and have different functionality. Actually the name is very appropriate "TestCase". One is Django test case the other is snapshot tests Django test case. The same way as a dog has a paw and a cat has a paw, but dog.paw differs from cat.paw .
Please do not think I am trying to split a hair here, I think it is very important. This is basically the purpose of namespaces.

[medmunds]

We have to take the whole namespace under consideration and then obviously it differs. [@jentyk]

That's an excellent point. I retract some of my earlier suggestions.

I'd rather have a flatter import structure than a nested one; if it's possible that all the things developers need can live in one namespace, it's simpler to talk about them. [@paulmelnikow]

I see the attraction, but it might be tricky to make all the things developers need fit in one namespace.

The snapshottest package currently supports either two or four testing frameworks (depending on how you count): pytest, unittest, and the django and nose derivatives of unittest. And it currently spreads its public API across a variety of namespaces, including the module root, framework-specific submodules, and other submodules. See list below.

At a minimum, it seems hard to avoid naming conflicts between the django and unittest submodules if everything developers need is re-exported at the module root. E.g., we'd end up with snapshottest.DjangoTestRunnerMixin and snapshottest.UnitTestTestRunnerMixin—and at that point, isn't it simpler to just stick with separate snapshottest.django and snapshottest.unittest namespaces?

aren't mixins basically a pattern for unittest.TestCase?

Good question; I don't know the answer. Mixins are a pattern for any class based code. Unittest is always class based. And I thought pytest was generally not used with class based tests, but then I saw in #53 a lot of people seem to be mixing the two.

---

FYI, here's a list of current and likely-near-future snapshottest public or semi-public APIs, showing the namespace where they're exposed:

• Things a Django user might need:
  • snapshottest.django.TestCase, SimpleTestCase (and possibly TransactionTestCase per this issue) (but all deprecated if we move to "mixin-only" approach)
  • snapshottest.TestCase (as a mixin for any unittest-derived test, also available as snapshottest.unittest.TestCase)
  • snapshottest.django.TestRunner, TestRunnerMixin
• Things a unittest user might need:
  • snapshottest.TestCase (also available as snapshottest.unittest.TestCase)
  • snapshottest.unittest.TextTestRunner, TestRunnerMixin, main (I'm about to add these in a PR to handle --snapshot-update and other missing session-level functionality)
• Things a pytest user might need:
  • snapshottest.pytest.PyTestSnapshotTest (see pytest --snapshot-update not working #53 (comment))
  • snapshottest.pyttest.snapshot (indirectly via pytest fixture magic and setup.py entry_points)
• Things a nose user might need:
  • snapshottest.TestCase (also available as snapshottest.unittest.TestCase and snapshottest.nose.TestCase)
  • snapshottest.nose.SnapshotTestPlugin (indirectly via setup.py entry_points)
• Things that could appear in a serialized snapshot file:
  • snapshottest.Snapshot
  • snapshottest.GenericRepr
  • snapshottest.file.FileSnapshot (a developer must also import this in their test code—see description in the pytest example under test_file())
• Semi-public APIs supporting advanced use cases or working with other testing frameworks:
  • snapshottest.formatter.Formatter.register_formatter()
  • snapshottest.assert_snapshot_match
  • snapshottest.module.SnapshotModule (this one is a much longer discussion for another time)
  • snapshottest.error.SnapshotError, SnapshotNotFound

[medmunds]

Also, no matter what we decide about namespaces, I do think we should expose something called SnapshotTestMixin for use with any of the unittest derived frameworks.

For whatever reason, a lot of developers seem to think in terms of, "I want something like my custom unittest case, but also with snapshottest capabilities mixed in":

from snapshottest import SnapshotTestMixin
from .my_test_utils import CustomTestCase

class MyTests(SnapshotTestMixin, CustomTestCase): ...

rather than "I want something that behaves like both a snapshottest TestCase and my custom unittest case":

import snapshottest
from .my_test_utils import CustomTestCase

class MyTests(snapshottest.TestCase, CustomTestCase): ...

(even though it's the exact same multiple inheritance, and snapshottest.SnapshotTestMixin is snapshottest.TestCase.)

If we go with per-framework namespaces, I'd (re-)export SnapshotTestMixin from all of snapshottest.unittest, snapshottest.django and snapshottest.nose.

[paulmelnikow]

I see the attraction, but it might be tricky to make all the things developers need fit in one namespace.

The snapshottest package currently supports either two or four testing frameworks (depending on how you count): pytest, unittest, and the django and nose derivatives of unittest. And it currently spreads its public API across a variety of namespaces, including the module root, framework-specific submodules, and other submodules. See list below.

At a minimum, it seems hard to avoid naming conflicts between the django and unittest submodules if everything developers need is re-exported at the module root. E.g., we'd end up with snapshottest.DjangoTestRunnerMixin and snapshottest.UnitTestTestRunnerMixin—and at that point, isn't it simpler to just stick with separate snapshottest.django and snapshottest.unittest namespaces?

To be honest, I don't think this is bad. I think not having to rely on the namespace makes it easier to talk about these classes, both in discussions and in documentation, and easier to verify that code is using the correct one. Also, imagine you've got a bunch of test modules using from snapshottest.django import TestCase. When you're creating a new module, it'd be an easy mistake to from snapshottest import TestCase. I think that error is way less likely if it's from snapshottest import DjangoTestCase.

If separate namespaces are desired, a possible compromise might be to keep the classes unique. e.g. snapshottest.django.DjangoTestCase

Really hoping we can find something that seems to work well all around.

Agreed about adding SnapshotTestMixin.

If we go with per-framework namespaces, I'd (re-)export SnapshotTestMixin from all of snapshottest.unittest, snapshottest.django and snapshottest.nose.

I'm in two minds about putting the same thing in multiple namespaces. It's confusing because some of the same-named things are the same (like SnapshotTestMixin), and others are different (like TestCase). Maybe best to sort out the larger question of namespace philosophy first, and then circle back to this one.

[paulmelnikow]

Mixins are a pattern for any class based code. Unittest is always class based. And I thought pytest was generally not used with class based tests, but then I saw in #53 a lot of people seem to be mixing the two.

Wow, I've never seen tests like that before!

I still am not super concerned about calling it SnapshotMixin or SnapshotTestMixin, as mixins aren't widely used with pytest, and we can document what it's intended for.