Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behaviour with data migration #330

Open
mschoettle opened this issue Dec 13, 2022 · 8 comments
Open

Inconsistent behaviour with data migration #330

mschoettle opened this issue Dec 13, 2022 · 8 comments

Comments

@mschoettle
Copy link

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.

@sobolevn
Copy link
Member

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?

@mschoettle
Copy link
Author

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).

@sobolevn
Copy link
Member

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

@mschoettle
Copy link
Author

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
Copy link
Author

I looked into pytest-django and found pytest-dev/pytest-django#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

@sobolevn
Copy link
Member

@mschoettle so, is this working as you expect?

@mschoettle
Copy link
Author

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

@skarzi
Copy link
Collaborator

skarzi commented Mar 25, 2023

@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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants