Inconsistent availability of data from migrations in TestCases when using --keepdb

[David Szotten]

I may have misunderstood something, but keepdb doesn't work as i expect when used with TransactionTestCase.

# models.py

class Model(models.Model):
    field = models.TextField()

# migrations

# 0001: default, as created by makemigrations
# 0002: datamigration

def create_data(apps, schema_editor):
    Model = apps.get_model("app", "Model")
    Model.objects.create(field='foo')


class Migration(migrations.Migration):

    dependencies = [
        ('app', '0001_initial'),
    ]

    operations = [
        migrations.RunPython(create_data)
    ]


# tests.py

from .models import Model

class Test(TransactionTestCase):
    def test_foo(self):
        self.assertEqual(Model.objects.count(), 1)

fails with --keepdb (then second time so feature is being used)

needs a non-sqlite db so as to not use :memory:

[Tim Graham]

A TransactionTestCase resets the database after the test runs by truncating all tables. For this reason, ​the docs say, "Any initial data loaded in migrations will only be available in TestCase tests and not in TransactionTestCase tests." I think that technically the data is available in the first TransactionTestCase (since truncation hasn't happened yet), but not any others. Perhaps this should be fixed or the docs clarified.

[Romain Garrigues]

I'm facing the same problem: my initial data are no more in the database at the end of my test suite, even with --keepdb option.
As TransactionTestCase is always flushing data on the tearDown, why not reloading the serialized data after the flush, instead of on the setUp step ?
I have tried a naive approach by moving the logic related to string serialized data loading on the tearDown step, ​https://github.com/romgar/django/commit/7219308cf8f196463d03d1407b0ad0e9b918a3db and the data is then kept.
Not really sure about any other impact anywhere else.
I run the django test suite without any failure.
Any feedbacks on that ?

[Tim Graham]

If you could submit a patch with that approach along with a regression test, I'll take a closer look.

[Romain Garrigues]

Great ! I will spend some time to create this regression test then.

[Romain Garrigues]

I had a quick look at django test suite and didn't managed to easily get where the keepdb option is tested.
I have seen admin_scripts.tests to launch some commands during tests and migration_test_data_persistance.tests related to TransactionTestCase data persistence.

A relevant test in that situation would be:

• launch the test command with --keepdb option (or even directly the TestRunner ?) over an app that contains initial data migrations and a TransactionTestCase,
• check after the return of this command if the database is still containing initial data migrations,
• clean the database that has been created through the test command.

I would appreciate some guidance and/or code references to help me.

[Tim Graham]

I think keepdb isn't an important part of testing this (it doesn't really have any tests). Instead you could use an approach similar to what's done in tests/test_utils/test_transactiontestcase.py where you invoke TransactionTestCase's methods within a test and check the availability of the data.

[Romain Garrigues]

Patch submitted ​https://github.com/django/django/pull/6137 with a regression test.

I will be glad to hear any comment about it.

I have created a new folder test_utils2 because I needed some migrations and didn't want to create the migrations related to already existing models in test_utils.

[Tim Graham]

Please uncheck "Patch needs improvement" when tests are passing.

[Romain Garrigues]

Can I do something else to help on that issue ?

[Tim Graham]

Find a colleague to review the patch.

[Moritz Sichert]

Left comments on the PR.

[Romain Garrigues]

Thanks @MoritzS for spending some of your time on it !!

[Tim Graham]

Left a few more comments. Please uncheck "Patch needs improvement" when the PR is updated.

[Romain Garrigues]

I have updated the rollback section and backward incompatible changes. Not really sure if if a good idea to add a "Changed in Django 1.9" and "Changed in Django 1.10" sections at the same time.

[Tim Graham]

The test isn't passing.

[Tim Graham]

$ ./tests/runtests.py postgres_tests --settings=test_postgres -k is crashing with the patch where it seems to work fine currently.

[Romain Garrigues]

I updated the MR according to my discussion with @aaugustin.
It impacted the DiscoverRunner explicit test ordering. I didn't want to update the current tests to ensure backward-compatibility, I created new ones for TransactionTestCase tests ordering.

[Tim Graham]

Some cosmetic comments for improvement are on the PR.

[jedie]

confirm this bug with Django==1.11.13

Any news, work-a-round for this?

[Romain Garrigues]

I would be keen to continue and have a look again, but I previously had a hard time finding reviewers for it.
There currently are only cosmetic changes requested by Tim Graham, and I would prefer to update them only when the approach used to fix the issue is approved first.

[Tim Graham <timograham@…>]

In b3b1d3d:

Fixed #25251 -- Made data migrations available in TransactionTestCase when using --keepdb.

Data loaded in migrations were restored at the beginning of each
TransactionTestCase and all the tables are truncated at the end of
these test cases. If there was a TransactionTestCase at the end of
the test suite, the migrated data weren't restored in the database
(especially unexpected when using --keepdb). Now data is restored
at the end of each TransactionTestCase.

[Tim Graham <timograham@…>]

In 9fa0d37:

Refs #25251 -- Filtered out skipped tests when processing the test suite to set _next_serialized_rollback.

[Tim Graham]

There's still an issue with this. Test methods that are decorated with skip doesn't have the __unittest_skip__ attribute set so the filtering of skipped tests doesn't work in that case (see ​PR comment). Since it seems a solution for that isn't forthcoming, I'm going to revert the patches to get the build that runs with --reverse passing again. I'll also close #29948 (a regression after the initial fix) which should be addressed if another patch for this issue is provided. ​PR

[Tim Graham <timograham@…>]

In 4c7c608:

Reverted "Fixed #25251 -- Made data migrations available in TransactionTestCase when using --keepdb."

This reverts commits b3b1d3d45fc066367f4fcacf0b06f72fcd00a9c6 and
9fa0d3786febf36c87ef059a39115aa1ce3326e8 due to reverse build failures
for which a solution isn't forthcoming.

[Markus Holtermann]

A few more insights in what's happening:

1. The issue occurs when TransactionTestCases are involved.
2. Data will be unavailable even in regular TestCases.

Let's assume an empty database. Additionally, the database must support transactions (e.g. PostgreSQL or MySQL (but not MyISAM)). Let's further say you have two test classes (TestRegular(TestCase) and TestTx(TransactionTestCase)) with the test methods TestRegular.test_1 and TestTx.test_2.

• When you run manage.py test --keepdb for the first time, Django will apply all migrations. That means, data created within those migrations will be present.

• Django runs tests inheriting from TestCase before those inheriting from TransactionTestCase (​https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/runner.py#L632 and ​https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/runner.py#L807-L832). That means, test_1() will be run before test_2().

• When data should be serialized for a database, Django will insert that data during the setup for a test function (​https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/testcases.py#L1005-L1012). This can be enabled explicitly for TransactionTestCase using serialized_rollback=True (​https://docs.djangoproject.com/en/3.1/topics/testing/overview/#rollback-emulation)

• When a test function in a TransactionTestCase is over, Django executes the flush management command (​https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/testcases.py#L1063-L1066). This empties the database.

• At the end of the test suite, however, Django does _not_ reinsert serialized test data, leaving the database empty after the first --keepdb run.

• Running manage.py test --keepdb again means, the database is empty (has no data that was previously created by the migrations). This means, tests in regular TestCases (e.g. test_1() will now fail as well.

[Markus Holtermann]

Here's a first draft at restoring data at the end of the test suite: ​https://github.com/django/django/pull/14147

[Blaž Šnuderl]

Anyone still looking into this? I would be interested to pick this up

[Markus Holtermann]

Feel free to take over ​https://github.com/django/django/pull/14147, Blaž. I'm not using TransactionTestCase anymore at the moment.

[Nathan Florea]

Sorry, I accidentally solved this in ​PR. I don't modify TransactionTestCase, though, so there's probably some other interesting work to do there. And I didn't technically fix --keepdb; I created a new flag, --use-clones, that only runs tests against clones of the test database. But it does provide an alternative to --keepdb that will always run tests against a fresh copy of the test database, ensuring that no matter what happens during testing or after, it will have no impact on subsequent test runs. We'll see if it gets accepted.