-
Notifications
You must be signed in to change notification settings - Fork 347
Support cleanup between tests with multiple databases #76
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
Comments
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. |
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 Here are some sample tests:
Here is the output with my patched
Here is the output with pytest_django's own
|
So from a quick looks it seems like a |
That sounds reasonable. I certainly don't have other ideas, better or otherwise. |
Hello. Is there any discussion about |
@slafs There has not been much discussions/requests for multi database support apart from this. It would be pretty simple to pass on |
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 Changing @ftobia's work-around to the following worked for me:
|
+1 👍 This is stopping us from switching from Django test runner to pytest. |
Why no use |
+1 |
1 similar comment
+1 |
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 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! |
Left some comments at #397 (comment), and created a PR, which would allow to change this in a generic way: #431. |
Another option to enable multi_db for those looking for a temporary solution. Put this in conftest.py
|
For anyone else looking for workarounds, note that
|
@jcushman do you mind sharing the full fixture or code snippet you used? |
For whatever reason, # 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. |
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 |
Adding reply in case anyone else will bump into this. You need to add the code below into the def pytest_sessionstart(session):
from django.test import TransactionTestCase
TransactionTestCase.databases = set(settings.DATABASES.keys()) |
So I believe this issue comes down to proper multi-db support. I'll close this issue as duplicate of that. |
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:Here is how I patched it as a work-around in my code:
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.
The text was updated successfully, but these errors were encountered: