From d82e789e027a9242b39e68b8cba10390478206e1 Mon Sep 17 00:00:00 2001 From: mistahchris Date: Tue, 15 Dec 2020 15:42:56 -0600 Subject: [PATCH 1/4] [MINOR] add opt-in multi-database support for db fixtures --- pytest_django/fixtures.py | 43 ++++++++++++++++++---- pytest_django_test/settings_sqlite.py | 7 ++++ pytest_django_test/settings_sqlite_file.py | 7 ++++ tests/test_database.py | 15 +++++++- 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/pytest_django/fixtures.py b/pytest_django/fixtures.py index 814dcbdf5..951fa7d1a 100644 --- a/pytest_django/fixtures.py +++ b/pytest_django/fixtures.py @@ -124,7 +124,7 @@ def teardown_database(): def _django_db_fixture_helper( - request, django_db_blocker, transactional=False, reset_sequences=False + request, django_db_blocker, settings, transactional=False, reset_sequences=False, ): if is_django_unittest(request): return @@ -148,11 +148,40 @@ class ResetSequenceTestCase(django_case): else: from django.test import TestCase as django_case + # specify attributes on test cases for multi-database support + # https://docs.djangoproject.com/en/3.1/topics/testing/tools/#multi-database-support + transactional_databases = _transactional_databases(settings) + if transactional_databases: + # django versions <= 1.8.X don't use `databases` attribute, it's all or nothing + # for those versions, django will create a transaction in every db in DATABASES + # if multi_db is True + # multi_db is not used in newer django versions + django_case.multi_db = True + try: + django_case.databases = django_case.databases.union(transactional_databases) + except AttributeError: + django_case.databases = transactional_databases + test_case = django_case(methodName="__init__") test_case._pre_setup() request.addfinalizer(test_case._post_teardown) +def _transactional_databases(settings): + """ + Get database labels from settings in which pytest-django should start a transaction. + """ + transactional_databases = {"default"} + for label, config in settings.DATABASES.items(): + if label == "default": + continue + + if config.get("TEST", {}).get("PYTEST_DJANGO_ALLOW_TRANSACTIONS"): + transactional_databases.add(label) + + return transactional_databases + + def _disable_native_migrations(): from django.conf import settings from django.core.management.commands import migrate @@ -196,7 +225,7 @@ def _set_suffix_to_test_databases(suffix): @pytest.fixture(scope="function") -def db(request, django_db_setup, django_db_blocker): +def db(request, django_db_setup, django_db_blocker, settings): """Require a django test database. This database will be setup with the default fixtures and will have @@ -218,11 +247,11 @@ def db(request, django_db_setup, django_db_blocker): ): request.getfixturevalue("transactional_db") else: - _django_db_fixture_helper(request, django_db_blocker, transactional=False) + _django_db_fixture_helper(request, django_db_blocker, settings, transactional=False) @pytest.fixture(scope="function") -def transactional_db(request, django_db_setup, django_db_blocker): +def transactional_db(request, django_db_setup, django_db_blocker, settings): """Require a django test database with transaction support. This will re-initialise the django database for each test and is @@ -237,11 +266,11 @@ def transactional_db(request, django_db_setup, django_db_blocker): """ if "django_db_reset_sequences" in request.fixturenames: request.getfixturevalue("django_db_reset_sequences") - _django_db_fixture_helper(request, django_db_blocker, transactional=True) + _django_db_fixture_helper(request, django_db_blocker, settings, transactional=True) @pytest.fixture(scope="function") -def django_db_reset_sequences(request, django_db_setup, django_db_blocker): +def django_db_reset_sequences(request, django_db_setup, django_db_blocker, settings): """Require a transactional test database with sequence reset support. This behaves like the ``transactional_db`` fixture, with the addition @@ -254,7 +283,7 @@ def django_db_reset_sequences(request, django_db_setup, django_db_blocker): ``transactional_db``, ``django_db_reset_sequences``. """ _django_db_fixture_helper( - request, django_db_blocker, transactional=True, reset_sequences=True + request, django_db_blocker, settings, transactional=True, reset_sequences=True ) diff --git a/pytest_django_test/settings_sqlite.py b/pytest_django_test/settings_sqlite.py index 8ace0293b..f6e48ea5c 100644 --- a/pytest_django_test/settings_sqlite.py +++ b/pytest_django_test/settings_sqlite.py @@ -4,5 +4,12 @@ "default": { "ENGINE": "django.db.backends.sqlite3", "NAME": "/should_not_be_accessed", + }, + "two": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": "/should_not_be_accessed_two", + "TEST": { + "PYTEST_DJANGO_ALLOW_TRANSACTIONS": True + } } } diff --git a/pytest_django_test/settings_sqlite_file.py b/pytest_django_test/settings_sqlite_file.py index a4e77ab11..bb7e16229 100644 --- a/pytest_django_test/settings_sqlite_file.py +++ b/pytest_django_test/settings_sqlite_file.py @@ -13,5 +13,12 @@ "ENGINE": "django.db.backends.sqlite3", "NAME": "/pytest_django_should_never_get_accessed", "TEST": {"NAME": _filename}, + }, + "secondary": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": "/should_not_be_accessed_two", + "TEST": { + "PYTEST_DJANGO_ALLOW_TRANSACTIONS": True + } } } diff --git a/tests/test_database.py b/tests/test_database.py index 4b5a0a5d5..5d71e4dad 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -1,5 +1,5 @@ import pytest -from django.db import connection +from django.db import connection, connections from django.test.testcases import connections_support_transactions from pytest_django_test.app.models import Item @@ -138,6 +138,19 @@ def test_fin(self, fin): # Check finalizer has db access (teardown will fail if not) pass + def test_multi_database_true_if_settings_permit_db(self, settings, db): + transactional_databases = set() + for label, config in settings.DATABASES.items(): + if config.get("TEST", {}).get("PYTEST_DJANGO_ALLOW_TRANSACTIONS"): + transactional_databases.add(label) + + if not transactional_databases: + pytest.skip( + "settings.DATABASES is not configured to create transactions" + "in databases outside of default" + ) + + assert all(conn.in_atomic_block for conn in connections.all()) class TestDatabaseFixturesAllOrder: @pytest.fixture From 5d0b03f1dcdeabf52f584901b740e21ce838fddb Mon Sep 17 00:00:00 2001 From: mistahchris Date: Tue, 15 Dec 2020 16:29:20 -0600 Subject: [PATCH 2/4] subclass django TestCase instead of modifying it, add docs --- docs/database.rst | 32 +++++++++++++++++++++----------- pytest_django/fixtures.py | 20 +++++++++++--------- tests/test_database.py | 8 ++++++-- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/docs/database.rst b/docs/database.rst index 2ed03ce8d..a1e66d3d8 100644 --- a/docs/database.rst +++ b/docs/database.rst @@ -66,22 +66,32 @@ select using an argument to the ``django_db`` mark:: Tests requiring multiple databases ---------------------------------- -Currently ``pytest-django`` does not specifically support Django's -multi-database support. +You can configure ``pytest-django`` to create transactions (or cleanup) databases +other than ``default`` by adding a special key to your database test configuration +in your django settings. + +for example:: + DATABASES = { + "default": { + # ... normal default settings + }, + "secondary": { + # ... engine settings and so forth + "TEST": { + "PYTEST_DJANGO_ALLOW_TRANSACTIONS": True + } + } + } + +With that database configuration in your test environment, tests that depend on the +``db`` or ``transactional_db`` fixtures can make changes in the "secondary" database +and have it cleaned up after each test. -You can however use normal :class:`~django.test.TestCase` instances to use its +You could also use normal :class:`~django.test.TestCase` instances to use its :ref:`django:topics-testing-advanced-multidb` support. In particular, if your database is configured for replication, be sure to read about :ref:`django:topics-testing-primaryreplica`. -If you have any ideas about the best API to support multiple databases -directly in ``pytest-django`` please get in touch, we are interested -in eventually supporting this but unsure about simply following -Django's approach. - -See `pull request 431 `_ -for an idea/discussion to approach this. - ``--reuse-db`` - reuse the testing database between test runs -------------------------------------------------------------- Using ``--reuse-db`` will create the test database in the same way as diff --git a/pytest_django/fixtures.py b/pytest_django/fixtures.py index 951fa7d1a..d8e4202c6 100644 --- a/pytest_django/fixtures.py +++ b/pytest_django/fixtures.py @@ -152,15 +152,17 @@ class ResetSequenceTestCase(django_case): # https://docs.djangoproject.com/en/3.1/topics/testing/tools/#multi-database-support transactional_databases = _transactional_databases(settings) if transactional_databases: - # django versions <= 1.8.X don't use `databases` attribute, it's all or nothing - # for those versions, django will create a transaction in every db in DATABASES - # if multi_db is True - # multi_db is not used in newer django versions - django_case.multi_db = True - try: - django_case.databases = django_case.databases.union(transactional_databases) - except AttributeError: - django_case.databases = transactional_databases + + class MultiDatabaseTransactionTestCase(django_case): + # django versions <= 1.8.X don't use `databases` attribute, it's all or nothing + # for those versions, django will create a transaction in every db in DATABASES + # if multi_db is True + # multi_db is not used in newer django versions + multi_db = True + databases = transactional_databases + + django_case = MultiDatabaseTransactionTestCase + request.config.django_transactional_databases = transactional_databases test_case = django_case(methodName="__init__") test_case._pre_setup() diff --git a/tests/test_database.py b/tests/test_database.py index 5d71e4dad..a96d98b93 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -138,7 +138,10 @@ def test_fin(self, fin): # Check finalizer has db access (teardown will fail if not) pass - def test_multi_database_true_if_settings_permit_db(self, settings, db): + def test_db_sets_transactions_in_all_configured_connections(self, settings, db): + assert all(conn.in_atomic_block for conn in connections.all()) + + def test_multi_database_true_if_settings_permit_db(self, request, settings, transactional_db): transactional_databases = set() for label, config in settings.DATABASES.items(): if config.get("TEST", {}).get("PYTEST_DJANGO_ALLOW_TRANSACTIONS"): @@ -150,7 +153,8 @@ def test_multi_database_true_if_settings_permit_db(self, settings, db): "in databases outside of default" ) - assert all(conn.in_atomic_block for conn in connections.all()) + assert {"default", "secondary"} == request.config.django_transactional_databases + class TestDatabaseFixturesAllOrder: @pytest.fixture From 0dc1b9b2f62dbb6643996691248e936c777ed47a Mon Sep 17 00:00:00 2001 From: mistahchris Date: Tue, 15 Dec 2020 16:35:34 -0600 Subject: [PATCH 3/4] only change settings for file sqlite to start --- pytest_django_test/settings_sqlite.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pytest_django_test/settings_sqlite.py b/pytest_django_test/settings_sqlite.py index f6e48ea5c..c36936a49 100644 --- a/pytest_django_test/settings_sqlite.py +++ b/pytest_django_test/settings_sqlite.py @@ -5,11 +5,4 @@ "ENGINE": "django.db.backends.sqlite3", "NAME": "/should_not_be_accessed", }, - "two": { - "ENGINE": "django.db.backends.sqlite3", - "NAME": "/should_not_be_accessed_two", - "TEST": { - "PYTEST_DJANGO_ALLOW_TRANSACTIONS": True - } - } } From aa87996b75b82c566b847c5080e8df7180d95fde Mon Sep 17 00:00:00 2001 From: Chris Cummings Date: Sat, 16 Jan 2021 15:16:09 -0600 Subject: [PATCH 4/4] remove django 1.8 support. fix test for myisam --- docs/database.rst | 2 ++ pytest_django/fixtures.py | 5 ----- tests/test_database.py | 5 ++++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/database.rst b/docs/database.rst index a1e66d3d8..2b94c897c 100644 --- a/docs/database.rst +++ b/docs/database.rst @@ -71,6 +71,7 @@ other than ``default`` by adding a special key to your database test configurati in your django settings. for example:: + DATABASES = { "default": { # ... normal default settings @@ -83,6 +84,7 @@ for example:: } } + With that database configuration in your test environment, tests that depend on the ``db`` or ``transactional_db`` fixtures can make changes in the "secondary" database and have it cleaned up after each test. diff --git a/pytest_django/fixtures.py b/pytest_django/fixtures.py index d8e4202c6..cd4ca8ef5 100644 --- a/pytest_django/fixtures.py +++ b/pytest_django/fixtures.py @@ -154,11 +154,6 @@ class ResetSequenceTestCase(django_case): if transactional_databases: class MultiDatabaseTransactionTestCase(django_case): - # django versions <= 1.8.X don't use `databases` attribute, it's all or nothing - # for those versions, django will create a transaction in every db in DATABASES - # if multi_db is True - # multi_db is not used in newer django versions - multi_db = True databases = transactional_databases django_case = MultiDatabaseTransactionTestCase diff --git a/tests/test_database.py b/tests/test_database.py index a96d98b93..d977a0349 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -139,7 +139,10 @@ def test_fin(self, fin): pass def test_db_sets_transactions_in_all_configured_connections(self, settings, db): - assert all(conn.in_atomic_block for conn in connections.all()) + assert all( + conn.in_atomic_block + for conn in connections.all() if conn.features.supports_transactions + ) def test_multi_database_true_if_settings_permit_db(self, request, settings, transactional_db): transactional_databases = set()