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

Support cleanup between tests with multiple databases #76

Closed
ftobia opened this issue Mar 20, 2014 · 20 comments
Closed

Support cleanup between tests with multiple databases #76

ftobia opened this issue Mar 20, 2014 · 20 comments

Comments

@ftobia
Copy link

ftobia commented Mar 20, 2014

pytest-django doesn't clean up between tests when using Django with multiple databases. The problem is related to this StackOverflow question: http://stackoverflow.com/questions/10121485/django-testcase-not-using-transactions-on-secondary-database

The db fixture uses Django's TestCase under the covers.

Here is the code from the original db fixture:

case = TestCase(methodName='__init__')
case._pre_setup()
request.addfinalizer(case._post_teardown)
request.addfinalizer(_django_cursor_wrapper.disable)

Here is how I patched it as a work-around in my code:

case = TestCase(methodName='__init__')
case.multi_db = True
case._pre_setup()
request.addfinalizer(case._post_teardown)
request.addfinalizer(_django_cursor_wrapper.disable)

Obviously that multi_db flag exists and defaults to False for a reason. I'm not sure the right way to incorporate multi_db support into pytest_django, and I'm not sure the right way to test such a change.

If you have a suggestion I can work on a pull request.

@flub
Copy link
Member

flub commented Mar 20, 2014

Could you give an example test using your patched version which uses two databases? I think that would help understand how multiple databases are used and what you need control over other then setting multi_db to true.

@ftobia
Copy link
Author

ftobia commented Mar 20, 2014

DATABASES = {
    'default': {
        'ENGINE'   : 'django.db.backends.sqlite3',
        'NAME'     : 'default.db',
    },
    'operations': {
        'ENGINE'   : 'django.db.backends.sqlite3',
        'NAME'     : 'operations.db',
    },
}

Most / all of my models are associated with the second database "operations". I believe Django's TestCase will flush transactions correctly on the default db, but not others unless multi_db=True.

Here are some sample tests:

from collection.models import Foo

def test_one(db):
    Foo.objects.create()
    assert Foo.objects.count() == 1

def test_two(db):
    assert Foo.objects.count() == 0

Here is the output with my patched db fixture, described above:

$ py.test multi_db_test/ -vv
======================================= test session starts =======================================
platform linux2 -- Python 2.7.2 -- py-1.4.20 -- pytest-2.5.2 -- bin/python
plugins: django
collected 2 items

multi_db_test/test_foo.py:3: test_one PASSED
multi_db_test/test_foo.py:7: test_two PASSED

==================================== 2 passed in 5.75 seconds =====================================

Here is the output with pytest_django's own db fixture:

$ py.test multi_db_test/ -vv
======================================= test session starts =======================================
platform linux2 -- Python 2.7.2 -- py-1.4.20 -- pytest-2.5.2 -- bin/python
plugins: django
collected 2 items

multi_db_test/test_foo.py:3: test_one PASSED
multi_db_test/test_foo.py:7: test_two FAILED

============================================ FAILURES =============================================
____________________________________________ test_two _____________________________________________

db = None

    def test_two(db):
>       assert Foo.objects.count() == 0
E       assert 1 == 0
E        +  where 1 = <bound method Manager.count of <django.db.models.manager.Manager object at 0x3cd0890>>()
E        +    where <bound method Manager.count of <django.db.models.manager.Manager object at 0x3cd0890>> = <django.db.models.manager.Manager object at 0x3cd0890>.count
E        +      where <django.db.models.manager.Manager object at 0x3cd0890> = Foo.objects

multi_db_test/test_foo.py:8: AssertionError
=============================== 1 failed, 1 passed in 5.85 seconds ===============================

@flub
Copy link
Member

flub commented Mar 25, 2014

So from a quick looks it seems like a multi_db fixture and pytest.mark.django_db(multi=True) could work as an API? Anyone else got any better ideas?

@ftobia
Copy link
Author

ftobia commented Mar 25, 2014

That sounds reasonable. I certainly don't have other ideas, better or otherwise.

@slafs
Copy link

slafs commented Oct 23, 2014

Hello. Is there any discussion about multi_db support besides this issue? The docs say that one should get in touch but whats the current status on this?

@pelme
Copy link
Member

pelme commented Jul 25, 2015

@slafs There has not been much discussions/requests for multi database support apart from this.

It would be pretty simple to pass on multi_db=True, but we also needs some kind of tests for this, so we also need a secondary database set up. If anyone is willing to work on this, it would be great :)

@robdennis
Copy link

I've been using the work-around @ftobia mentions in the opening post, but after upgrading to Django 1.8.4 from 1.7 it stopped working.

There was Django change to make the code handling setup/tearDown look for a multi_db class attribute instead of an instance attribute.

Changing @ftobia's work-around to the following worked for me:

case = TestCase(methodName='__init__')
case.__class__.multi_db = True
case._pre_setup()
request.addfinalizer(case._post_teardown)
request.addfinalizer(_django_cursor_wrapper.disable)

@adrianandreias
Copy link

+1 👍

This is stopping us from switching from Django test runner to pytest.

@eremeevdev
Copy link

Why no use django_case.multi_db = True here https://github.com/pytest-dev/pytest-django/blob/master/pytest_django/fixtures.py#L107 by default?

@knabben
Copy link

knabben commented Jun 28, 2016

+1

1 similar comment
@guilhermemaba
Copy link

+1

@pelme
Copy link
Member

pelme commented Jun 28, 2016

I think that having a marker on the django_db marker like would by I good API:

@pytest.mark.django_db(transactional=True, multi_db=True)
def test_a():
    pass

The implementation should be relatively similar to reset_sequences (PR #308) or serialized_rollback (PR #353).

The "tricky" part would be to improve pytest-django's internal test suite to contain multiple databases.

If someone wants to work in this I will for sure review it and help in getting it merged. Feel free to start working on a PR if this is something that would be interesting to you!

@blueyed
Copy link
Contributor

blueyed commented Nov 21, 2016

Left some comments at #397 (comment), and created a PR, which would allow to change this in a generic way: #431.

@cheniel
Copy link

cheniel commented Feb 7, 2018

Another option to enable multi_db for those looking for a temporary solution. Put this in conftest.py

def pytest_sessionstart(session):
    from django.test import TransactionTestCase
    TransactionTestCase.multi_db = True

@jcushman
Copy link

For anyone else looking for workarounds, note that multi_db was deprecated in Django 2.2. The replacement that worked for me is:

TransactionTestCase.databases = set(settings.DATABASES.keys())

@merit-mthompson
Copy link

@jcushman do you mind sharing the full fixture or code snippet you used?

@merit-mthompson
Copy link

merit-mthompson commented Aug 12, 2020

For whatever reason, pytest_sessionstart() in our base conftest.py wasn't doing anything. Took a page from #342 along with standard pytest monkeypatch stuff and have landed on the following for our multi-database use:

# OPTION 1: Function fixture; must be included with tests
@pytest.fixture
def django_db_multiple(monkeypatch, request, settings):
    """
    Ensure all test functions using Django test cases have multiple database
    support. This is mostly/only so that Django will wrap ALL database use with
    atomic blocks like it does for DEFAULT_DB_ALIAS.

    https://github.com/django/django/blob/master/django/test/testcases.py#L903
    https://github.com/pytest-dev/pytest-django/issues/76
    """
    from django.test import TestCase
    from django.test import TransactionTestCase

    db_keys = set(settings.DATABASES.keys())

    monkeypatch.setattr(TestCase, 'databases', db_keys)
    monkeypatch.setattr(TransactionTestCase, 'databases', db_keys)

@pytest.mark.django_db
def test_some_test(django_db_multiple):
    pass

# OPTION 2: Session fixture
@pytest.fixture(autouse=True, scope='session')
def django_db_multiple():
    """
    Ensure all test functions using Django test cases have multiple database
    support. This is mostly/only so that Django will wrap ALL database use with
    atomic blocks like it does for DEFAULT_DB_ALIAS.

    https://github.com/django/django/blob/master/django/test/testcases.py#L903
    https://github.com/pytest-dev/pytest-django/issues/76
    https://github.com/pytest-dev/pytest/issues/1872
    """
    from _pytest.monkeypatch import MonkeyPatch
    from django.test import TestCase
    from django.test import TransactionTestCase
    from django.conf import settings

    db_keys = set(settings.DATABASES.keys())

    monkeypatch = MonkeyPatch()
    monkeypatch.setattr(TestCase, 'databases', db_keys)
    monkeypatch.setattr(TransactionTestCase, 'databases', db_keys)

    yield monkeypatch

    monkeypatch.undo()

Edit: we moved to a session fixture.

@theoden-dd
Copy link

theoden-dd commented Oct 23, 2020

In order not to loose pytest superpowers by switching to unittest Django test cases, I copied and patched pytest-django internals to this ugly (yet working!) hack:

from typing import Optional, Type, TypeVar

import pytest
from django.test import TransactionTestCase
from pytest_django.django_compat import is_django_unittest

TestCase = TypeVar('TestCase', bound=TransactionTestCase)


def _django_db_fixture_helper(
    request, django_db_blocker, transactional: bool = False, reset_sequences: bool = False,
) -> Optional[Type[TestCase]]:
    if is_django_unittest(request):
        return None

    if not transactional and 'live_server' in request.fixturenames:
        # Do nothing, we get called with transactional=True, too.
        return None

    django_db_blocker.unblock()
    request.addfinalizer(django_db_blocker.restore)

    if transactional:
        from django.test import TransactionTestCase as DjangoTestCase  # noqa: WPS433

        if reset_sequences:

            class ResetSequenceTestCase(DjangoTestCase):  # noqa: WPS431
                reset_sequences = True

            DjangoTestCase = ResetSequenceTestCase  # type: ignore[misc] # noqa: N806
    else:
        from django.test import TestCase as DjangoTestCase  # type: ignore[no-redef] # noqa: WPS433

    return DjangoTestCase  # type: ignore[return-value]


@pytest.fixture()
def db_case(request, django_db_setup, django_db_blocker):
    """Require a django test database.

    This database will be setup with the default fixtures and will have
    the transaction management disabled. At the end of the test the outer
    transaction that wraps the test itself will be rolled back to undo any
    changes to the database (in case the backend supports transactions).
    This is more limited than the ``transactional_db`` resource but
    faster.

    If multiple database fixtures are requested, they take precedence
    over each other in the following order (the last one wins): ``db``,
    ``transactional_db``, ``django_db_reset_sequences``.
    """
    if 'django_db_reset_sequences' in request.fixturenames:
        request.getfixturevalue('django_db_reset_sequences')
    if (
        'transactional_db' in request.fixturenames
        or 'live_server' in request.fixturenames
    ):
        request.getfixturevalue('transactional_db')
    else:
        django_case: Optional[Type[TransactionTestCase]] = _django_db_fixture_helper(
            request, django_db_blocker, transactional=False,
        )

        def factory(dbs=None):  # noqa: WPS430
            if django_case is None:
                return
            CaseType: Type[TransactionTestCase] = django_case  # noqa: N806
            if dbs is not None:
                class DatabasesSetTestCase(  # noqa: WPS431
                    CaseType,  # type: ignore[valid-type, misc]
                ):
                    databases = dbs
                CaseType = DatabasesSetTestCase  # noqa: N806
            test_case: TransactionTestCase = CaseType(methodName='__init__')
            test_case._pre_setup()  # type: ignore[attr-defined] # noqa: WPS437
            request.addfinalizer(
                test_case._post_teardown,  # type: ignore[attr-defined] # noqa: WPS437
            )

        return factory

With it one can use something like:

class TestCase(object):

    def test_ok(
        self,
        db_case,
        ...
    ):
        db_case(dbs=('non_default_db_alias',))

This should be used instead of db fixture or pytest.mark.django_db mark.

@roks0n
Copy link

roks0n commented Feb 26, 2021

@jcushman do you mind sharing the full fixture or code snippet you used?

Adding reply in case anyone else will bump into this.

You need to add the code below into the conftest.py. This fixture gets picked up automatically so no need to add it to a test.

def pytest_sessionstart(session):
    from django.test import TransactionTestCase

    TransactionTestCase.databases = set(settings.DATABASES.keys())

@bluetech
Copy link
Member

bluetech commented May 7, 2021

So I believe this issue comes down to proper multi-db support. I'll close this issue as duplicate of that.

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