Tests using live_server fixture removing data from data migrations

[ekiro]

I've created a simple test case to reproduce this behavior https://github.com/ekiro/case_pytest/blob/master/app/tests.py which fails after second test using live_server fixture.
MyModel objects are created in migration, using RunPython. It seems like after any test with live_server, every row from the database is truncated. Both, postgresql and sqlite3 was tested.

EDIT:
Tests

"""
MyModel objects are created in migration
Test results:
    app/tests.py::test_no_live_server PASSED
    app/tests.py::test_live_server PASSED
    app/tests.py::test_live_server2 FAILED
    app/tests.py::test_no_live_server_after_live_server FAILED
"""

import pytest

from .models import MyModel


@pytest.mark.django_db()
def test_no_live_server():
    """Passed"""
    assert MyModel.objects.count() == 10


@pytest.mark.django_db()
def test_live_server(live_server):
    """Passed"""
    assert MyModel.objects.count() == 10


@pytest.mark.django_db()
def test_live_server2(live_server):
    """Failed, because count() returns 0"""
    assert MyModel.objects.count() == 10


@pytest.mark.django_db()
def test_no_live_server_after_live_server():
    """Failed, because count() returns 0"""
    assert MyModel.objects.count() == 10

[blueyed]

That's because of the transactional_db fixture being used automatically by live_server (there is no need to mark it with @pytest.mark.django_db). There are several issues / discussions in this regard, but the last time I looked closer at this, there was no easy solution to this problem that comes with using data migrations.
A workaround is to use pytest fixtures to wrap your data migrations / data fixtures.
(btw: better put the tests inline in the issue rather than into an external resource that might change over time.)

[aaugustin]

I just hit this as well and it took me a long time to track it down, even though I'm reasonably familiar with Django's test framework and migrations.

This is actually a surprising performance tradeoff of Django's TestCase. It's documented here:
https://docs.djangoproject.com/en/1.9/topics/testing/overview/#rollback-emulation

In a regular Django test suite, the workaround consists in setting a serialized_rollback = True class attribute on the test case.

I don't know how to achieve the same effect with pytest-django's dynamically generated test classes.

[aaugustin]

The following change "solves" the issue at the expense of unconditionally selecting the least efficient behavior.

--- pytest_django/fixtures.py.orig  2016-04-27 17:12:25.000000000 +0200
+++ pytest_django/fixtures.py   2016-04-27 17:21:50.000000000 +0200
@@ -103,6 +103,7 @@

     if django_case:
         case = django_case(methodName='__init__')
+        case.serialized_rollback = True
         case._pre_setup()
         request.addfinalizer(case._post_teardown)

[aaugustin]

The following technique works, but I can't recommend it for rather obvious reasons...

import pytest
from django.core.management import call_command
from pytest_django.fixtures import transactional_db as _transactional_db


def _reload_fixture_data():
    fixture_names = [
        # Create fixtures for the data created by data migrations
        # and list them here.
    ]
    call_command('loaddata', *fixture_names)


@pytest.fixture(scope='function')
def transactional_db(request, _django_db_setup, _django_cursor_wrapper):
    """
    Override a pytest-django fixture to restore the contents of the database.

    This works around https://github.com/pytest-dev/pytest-django/issues/329 by
    restoring data created by data migrations. We know what data matters and we
    maintain it in (Django) fixtures. We don't read it from the database. This
    causes some repetition but keeps this (pytest) fixture (almost) simple.

    """
    try:
        return _transactional_db(request, _django_db_setup, _django_cursor_wrapper)
    finally:
        # /!\ Epically shameful hack /!\ _transactional_db adds two finalizers:
        # _django_cursor_wrapper.disable() and case._post_teardown(). Note that
        # finalizers run in the opposite order of that in which they are added.
        # We want to run after case._post_teardown() which flushes the database
        # but before _django_cursor_wrapper.disable() which prevents further
        # database queries. Hence, open heart surgery in pytest internals...
        finalizers = request._fixturedef._finalizer
        assert len(finalizers) == 2
        assert finalizers[0].__qualname__ == 'CursorManager.disable'
        assert finalizers[1].__qualname__ == 'TransactionTestCase._post_teardown'
        finalizers.insert(1, _reload_fixture_data)

[pelme]

Would an option to conditionally enable serialized_rollback be a good solution?

I.e. something like

@pytest.mark.django_db(transaction=True, serialized_rollback=True)
def test_foo():
    ...

[aaugustin]

I think that would be good.

In my use case:

• the transactional_db fixture is triggered by live_server, but I wouldn't mind adding a django_db fixture to specify serialized rollback
• unrelated to pytest-django -- case.serialized_rollback = True still fails because Django attemps to deserialized objects in an order that doesn't respect FK constraints, I think that's a bug in Django

[pelme]

Ok, perfect :)

I don't have time to work on a fix right now, but adding such an option to the django_db marker should and setting case.serialized_rollback = True (like you did above) should be relatively straightforward.

[aaugustin]

Ticket for the Django bug I mentioned above: https://code.djangoproject.com/ticket/26552

[aaugustin]

@pelme -- does this pull request implement the "relatively straightforward" approach you're envisionning?

[pelme]

Thanks a lot for the PR. The implementation looks correct and what I imagined. I did not think of having a serialized_rollback fixture but it is indeed useful to be able to force this behaviour from other fixtures.

We would need a test for this too, but the implementation looks correct!

[aaugustin]

I'll try to find time to polish the patch (I haven't even run it yet). I need to get the aformentionned bug in Django fixed as well before I can use this.

[aaugustin]

Updated version of the hack above for pytest-django ≥ 3.0:

@pytest.fixture(scope='function')
def transactional_db(request, django_db_setup, django_db_blocker):
    """
    Override a pytest-django fixture to restore the contents of the database.

    This works around https://github.com/pytest-dev/pytest-django/issues/329 by
    restoring data created by data migrations. We know what data matters and we
    maintain it in (Django) fixtures. We don't read it from the database. This
    causes some repetition but keeps this (pytest) fixture (almost) simple.

    """
    try:
        return _transactional_db(request, django_db_setup, django_db_blocker)
    finally:
        # /!\ Epically shameful hack /!\ _transactional_db adds two finalizers:
        # django_db_blocker.restore() and test_case._post_teardown(). Note that
        # finalizers run in the opposite order of that in which they are added.
        # We want to run after test_case._post_teardown() flushes the database
        # but before django_db_blocker.restore() prevents further database
        # queries. Hence, open heart surgery in pytest internals...
        finalizers = request._fixturedef._finalizer
        assert len(finalizers) == 2
        assert finalizers[0].__qualname__ == '_DatabaseBlocker.restore'
        assert finalizers[1].__qualname__ == 'TransactionTestCase._post_teardown'
        finalizers.insert(1, _reload_fixture_data)

[pelme]

Thanks for the update and sorry I couldn't get this into 3.0. I will try to get to this for the next release!

[Wierrat]

Is there any chance to see it in next release?

Thanks

[blueyed]

@Wierrat
Yes, probably.

Can help us with a test for #353 before that then please?

[atodorov]

Hi folks,
what is the latest status of this? In particular I'm using pytest as my test runner with the StaticLiveServerTestCase class from Django. I've set the class attribute serialized_rollback = True but that seems to be in effect only when executing the first test from the sequence.

[telenieko]

Just got caught by this one. Looking around it seems there have been some PR on this issue but none of them appear to have been merged.
Is there any particular reason this issue remain open?

[blueyed]

some PR

Likely/only #329, no?

The last time I've asked about help with a test, and it has several conflicts (maybe only due to black).

I still suggest for anyone affected to help with that - I am not using it myself.

[telenieko]

Thanks for the quick reply @blueyed.
My knowledge of pytest / pytest-django is minimal but I'll put that on my somday/maybe list! :D

[hannahdiels]

Updated version of the above hack for pytest 5.2.1, pytest-django 3.5.1:

from functools import partial

@pytest.fixture
def transactional_db(request, transactional_db, django_db_blocker):
    # Restore DB content after all of transactional_db's finalizers have
    # run. Finalizers are run in the opposite order of that in which they
    # are added, so we prepend the restore to the front of the list.
    #
    # Works for pytest 5.2.1, pytest-django 3.5.1
    restore = partial(_restore_db_content, django_db_blocker)
    finalizers = request._fixture_defs['transactional_db']._finalizers
    finalizers.insert(0, restore)

    # Simply restoring after yielding transactional_db wouldn't work because
    # it would run before transactional_db's finalizers which contains the truncate.
    return transactional_db

def _restore_db_content(django_db_fixture, django_db_blocker):
    with django_db_blocker.unblock():
        call_command('loaddata', '--verbosity', '0', 'TODO your fixture')

I had to depend on the original transactional_db fixture instead of calling it as pytest no longer allows calling fixture functions directly. I then get the fixture def of the original fixture and prepend a finalizer to it. While inserting at index 1 still worked, I insert before all finalizers and use django_db_blocker in the restore function as that seems slightly less fragile.

Edit: removed unnecessary try finally.

[625781186]

@lukaszb

#848

I use this version of pytest-django (https://github.com/pytest-dev/pytest-django/pull/721/files),
But still error.

[aaugustin]

While sorting my archives, I stumbled upon this attempt, which is probably not very useful but who knows. So here it is.

commit c1a6a3fe939d9516ec7cabe1dfd6903c512db22a
Author: Aymeric Augustin <[email protected]>
Date:   Thu Apr 28 11:51:06 2016 +0200

    Add support for serialized rollback. Fix #329.
    
    WORK IN PROGRESS - UNTESTED

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 9b4c510..c50981c 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -3,6 +3,15 @@ Changelog
 
 NEXT
 ----
+
+Features
+^^^^^^^^
+* Add support for serialized rollback in transactional tests.
+  Thanks to Piotr Karkut for `the bug report
+  <https://github.com/pytest-dev/pytest-django/issues/329>`_.
+
+Bug fixes
+^^^^^^^^^
 * Fix error when Django happens to be imported before pytest-django runs.
   Thanks to Will Harris for `the bug report
   <https://github.com/pytest-dev/pytest-django/issues/289>`_.
diff --git a/docs/helpers.rst b/docs/helpers.rst
index 7c60f90..1860ee1 100644
--- a/docs/helpers.rst
+++ b/docs/helpers.rst
@@ -16,7 +16,7 @@ on what marks are and for notes on using_ them.
 ``pytest.mark.django_db`` - request database access
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. py:function:: pytest.mark.django_db([transaction=False])
+.. py:function:: pytest.mark.django_db([transaction=False, serialized_rollback=False])
 
    This is used to mark a test function as requiring the database. It
    will ensure the database is setup correctly for the test. Each test
@@ -38,6 +38,14 @@ on what marks are and for notes on using_ them.
      uses. When ``transaction=True``, the behavior will be the same as
      `django.test.TransactionTestCase`_
 
+   :type serialized_rollback: bool
+   :param serialized_rollback:
+     The ``serialized_rollback`` argument enables `rollback emulation`_.
+     After a `django.test.TransactionTestCase`_ runs, the database is
+     flushed, destroying data created in data migrations. This is the
+     default behavior of Django. Setting ``serialized_rollback=True``
+     tells Django to restore that data.
+
    .. note::
 
       If you want access to the Django database *inside a fixture*
@@ -54,6 +62,7 @@ on what marks are and for notes on using_ them.
      Test classes that subclass Python's ``unittest.TestCase`` need to have the
      marker applied in order to access the database.
 
+.. _rollback emulation: https://docs.djangoproject.com/en/stable/topics/testing/overview/#rollback-emulation
 .. _django.test.TestCase: https://docs.djangoproject.com/en/dev/topics/testing/overview/#testcase
 .. _django.test.TransactionTestCase: https://docs.djangoproject.com/en/dev/topics/testing/overview/#transactiontestcase
 
@@ -191,6 +200,16 @@ transaction support.  This is only required for fixtures which need
 database access themselves.  A test function would normally use the
 :py:func:`~pytest.mark.django_db` mark to signal it needs the database.
 
+``serialized_rollback``
+~~~~~~~~~~~~~~~~~~~~~~~
+
+When the ``transactional_db`` fixture is enabled, this fixture can be
+added to trigger `rollback emulation`_ and thus restores data created
+in data migrations after each transaction test.  This is only required
+for fixtures which need to enforce this behavior.  A test function
+would use :py:func:`~pytest.mark.django_db(serialized_rollback=True)`
+to request this behavior.
+
 ``live_server``
 ~~~~~~~~~~~~~~~
 
@@ -200,6 +219,12 @@ or by requesting it's string value: ``unicode(live_server)``.  You can
 also directly concatenate a string to form a URL: ``live_server +
 '/foo``.
 
+Since the live server and the tests run in different threads, they
+cannot share a database transaction. For this reason, ``live_server``
+depends on the ``transactional_db`` fixture. If tests depend on data
+created in data migrations, you should add the ``serialized_rollback``
+fixture.
+
 ``settings``
 ~~~~~~~~~~~~
 
diff --git a/pytest_django/fixtures.py b/pytest_django/fixtures.py
index cae7d47..ca2fc72 100644
--- a/pytest_django/fixtures.py
+++ b/pytest_django/fixtures.py
@@ -64,7 +64,8 @@ def _django_db_setup(request,
         request.addfinalizer(teardown_database)
 
 
-def _django_db_fixture_helper(transactional, request, _django_cursor_wrapper):
+def _django_db_fixture_helper(transactional, serialized_rollback,
+                              request, _django_cursor_wrapper):
     if is_django_unittest(request):
         return
 
@@ -105,6 +106,7 @@ def _django_db_fixture_helper(transactional, request, _django_cursor_wrapper):
 
     if django_case:
         case = django_case(methodName='__init__')
+        case.serialized_rollback = serialized_rollback
         case._pre_setup()
         request.addfinalizer(case._post_teardown)
 
@@ -177,7 +179,9 @@ def db(request, _django_db_setup, _django_cursor_wrapper):
     if 'transactional_db' in request.funcargnames \
             or 'live_server' in request.funcargnames:
         return request.getfuncargvalue('transactional_db')
-    return _django_db_fixture_helper(False, request, _django_cursor_wrapper)
+    return _django_db_fixture_helper(
+        transaction=False, serialized_rollback=False,
+        request=request, _django_cursor_wrapper=_django_cursor_wrapper)
 
 
 @pytest.fixture(scope='function')
@@ -192,7 +196,22 @@ def transactional_db(request, _django_db_setup, _django_cursor_wrapper):
     database setup will behave as only ``transactional_db`` was
     requested.
     """
-    return _django_db_fixture_helper(True, request, _django_cursor_wrapper)
+    # TODO -- is request.getfuncargvalue('serialized_rollback') enough
+    #         to add 'serialized_rollback' to request.funcargnames?
+    serialized_rollback = 'serialized_rollback' in request.funcargnames
+    return _django_db_fixture_helper(
+        transaction=True, serialized_rollback=serialized_rollback,
+        request=request, _django_cursor_wrapper=_django_cursor_wrapper)
+
+
+@pytest.fixture(scope='function')
+def serialized_rollback(request):
+    """Enable serialized rollback after transaction test cases
+
+    This fixture only has an effect when the ``transactional_db``
+    fixture is active, which happen as a side-effect of requesting
+    ``live_server``.
+    """
 
 
 @pytest.fixture()
diff --git a/pytest_django/plugin.py b/pytest_django/plugin.py
index d499e6f..204f16f 100644
--- a/pytest_django/plugin.py
+++ b/pytest_django/plugin.py
@@ -372,6 +372,8 @@ def _django_db_marker(request):
             request.getfuncargvalue('transactional_db')
         else:
             request.getfuncargvalue('db')
+        if marker.serialized_rollback:
+            request.getfuncargvalue('serialized_rollback')
 
 
 @pytest.fixture(autouse=True, scope='class')
@@ -564,8 +566,9 @@ def validate_django_db(marker):
     It checks the signature and creates the `transaction` attribute on
     the marker which will have the correct value.
     """
-    def apifun(transaction=False):
+    def apifun(transaction=False, serialized_rollback=False):
         marker.transaction = transaction
+        marker.serialized_rollback = serialized_rollback
     apifun(*marker.args, **marker.kwargs)
 
 

[bluetech]

A PR for this is in #970. I intend to merge and release it in a few days. Comments and tests are welcome.