django-test-migrations icon indicating copy to clipboard operation
django-test-migrations copied to clipboard

Inconsistent behaviour with data migration

Open mschoettle opened this issue 2 years ago • 8 comments

We added a data migration and are testing it using django-test-migrations. When running pytest with --create-db everything works fine. But when running it with --reuse-db the data added by the data migration is gone. Since this only happens when migration tests are part of the suite it seems that this is something that happens in this plugin.

pytest 7.1.3 django-test-migrations-1.2.0

Is there anything that could trigger this? Maybe: https://github.com/wemake-services/django-test-migrations/blob/master/django_test_migrations/migrator.py#L47

Probably unrelated but I noticed that Migrator.reset() causes the last migration to be re-applied (after a migration test that starts with the second last and applies the last one):

migrator.apply_initial_migration(('foo', '0001_...'))
migrator.apply_tested_migration(('foo', '0002_...'))

The call to the migrate command will cause 0002 to be re-applied. If 0002 is a data migration it will attempt to add the same data again.

mschoettle avatar Dec 13 '22 03:12 mschoettle

I think that using --reuse-db with this plugin is problematic.

Can you run two tests commands? One with --create-db and this plugin, other with other tests and --reuse-db?

sobolevn avatar Dec 13 '22 08:12 sobolevn

That might be possible. Or somehow excluding those tests via @pytest.mark.skipif when running with --reuse-db.

In either case, do you know which part of this plugin could cause this? It seems to happen as soon as the migrator fixture is used by a test function (even when not using it within the test).

mschoettle avatar Dec 14 '22 02:12 mschoettle

No, I don't know. But, if you can figure that out, PR is totally welcome!

sobolevn avatar Dec 14 '22 05:12 sobolevn

I tracked this down to the use of the transactional_db fixture here https://github.com/wemake-services/django-test-migrations/blob/master/django_test_migrations/contrib/pytest_plugin.py#L32

Removing it does not work. The tests then fail with:

django.db.utils.OperationalError: cannot ALTER TABLE "<table>" because it has pending trigger events

mschoettle avatar Dec 17 '22 21:12 mschoettle

I looked into pytest-django and found https://github.com/pytest-dev/pytest-django/pull/970 which added support for serialized rollback. I added django_db_serialized_rollback to the fixture.

It ran fine with --reuse-db the first time but after running it once with --create-db the data was gone again.

See: https://pytest-django.readthedocs.io/en/latest/helpers.html#std-fixture-django_db_serialized_rollback and https://docs.djangoproject.com/en/stable/topics/testing/overview/#test-case-serialized-rollback

mschoettle avatar Dec 17 '22 21:12 mschoettle

@mschoettle so, is this working as you expect?

sobolevn avatar Dec 18 '22 07:12 sobolevn

One of our developers ran into this issue again despite the change to always use --create-db. We use pytest-randomly and as soon as the migration tests run before other tests that require that data to be there it fails.

I realized today that the execution order of tests can be modified. In combination with --create-db this should fix it completely:

from _pytest.config import Config
from _pytest.main import Session
from _pytest.python import Function, Module

def pytest_collection_modifyitems(session: Session, config: Config, items: list[Function]) -> None:
    """
    Change the execution order of tests.

    Ensure that migration tests run last.
    This is to avoid the migrator from `django-test-migrations` to cause data migrations to be flushed.
    See: https://github.com/wemake-services/django-test-migrations/issues/330
  
    See pytest docs: https://docs.pytest.org/en/latest/reference/reference.html#pytest.hookspec.pytest_collection_modifyitems

    Args:
        session: the pytest session
        config: the pytest configuration
        items: the items to test (test functions and test classes)
    """
    migration_tests = []
    original_items_order = []

    for item in items:
        # some test functions are wrapped within a class
        module = item.getparent(Module)

        if module and module.name == 'test_migrations.py':
            migration_tests.append(item)
        else:
            original_items_order.append(item)

    # modify items in place with migration tests moved to the end
    items[:] = original_items_order + migration_tests

mschoettle avatar Feb 14 '23 20:02 mschoettle

@mschoettle have you tried to run migrations tests separately from other tests? You can run just migrations tests with the following command:

pytest -m migration_test

and to run all other tests:

pytest -m not migration_test
# or
pytest --no-migrations

I know it's a bit more problematic, because of 2 separate processes (runs) of pytest, but it's pretty hard to maintain the DB state when running migrations tests.

Reference: https://github.com/wemake-services/django-test-migrations#choosing-only-migrations-tests

BTW I will experiment a bit with serialized rollback, maybe it will help somehow :+1:

skarzi avatar Mar 25 '23 13:03 skarzi