Fixed #31395 -- Made setUpTestData enforce in-memory data isolation.

Since it's introduction in Django 1.8 setUpTestData has been suffering
from a documented but confusing caveat due to its sharing of attributes
assigned during its execution with all test instances.

By keeping track of class attributes assigned during the setUpTestData
phase its possible to ensure only deep copies are provided to test
instances on attribute retreival and prevent manual setUp gymnastic to
work around the previous lack of in-memory data isolation.

Thanks Adam Johnson for the extensive review.
This commit is contained in:
Simon Charette 2018-11-23 21:22:09 -05:00 committed by Mariusz Felisiak
parent 1dd96f731d
commit 3cf80d3fcf
6 changed files with 178 additions and 12 deletions

View File

@ -5,9 +5,10 @@ import posixpath
import sys import sys
import threading import threading
import unittest import unittest
import warnings
from collections import Counter from collections import Counter
from contextlib import contextmanager from contextlib import contextmanager
from copy import copy from copy import copy, deepcopy
from difflib import get_close_matches from difflib import get_close_matches
from functools import wraps from functools import wraps
from unittest.suite import _DebugResult from unittest.suite import _DebugResult
@ -40,6 +41,7 @@ from django.test.utils import (
CaptureQueriesContext, ContextList, compare_xml, modify_settings, CaptureQueriesContext, ContextList, compare_xml, modify_settings,
override_settings, override_settings,
) )
from django.utils.deprecation import RemovedInDjango41Warning
from django.utils.functional import classproperty from django.utils.functional import classproperty
from django.views.static import serve from django.views.static import serve
@ -1071,6 +1073,59 @@ def connections_support_transactions(aliases=None):
return all(conn.features.supports_transactions for conn in conns) return all(conn.features.supports_transactions for conn in conns)
class TestData:
"""
Descriptor to provide TestCase instance isolation for attributes assigned
during the setUpTestData() phase.
Allow safe alteration of objects assigned in setUpTestData() by test
methods by exposing deep copies instead of the original objects.
Objects are deep copied using a memo kept on the test case instance in
order to maintain their original relationships.
"""
memo_attr = '_testdata_memo'
def __init__(self, name, data):
self.name = name
self.data = data
def get_memo(self, testcase):
try:
memo = getattr(testcase, self.memo_attr)
except AttributeError:
memo = {}
setattr(testcase, self.memo_attr, memo)
return memo
def __get__(self, instance, owner):
if instance is None:
return self.data
memo = self.get_memo(instance)
try:
data = deepcopy(self.data, memo)
except TypeError:
# RemovedInDjango41Warning.
msg = (
"Assigning objects which don't support copy.deepcopy() during "
"setUpTestData() is deprecated. Either assign the %s "
"attribute during setUpClass() or setUp(), or add support for "
"deepcopy() to %s.%s.%s."
) % (
self.name,
owner.__module__,
owner.__qualname__,
self.name,
)
warnings.warn(msg, category=RemovedInDjango41Warning, stacklevel=2)
data = self.data
setattr(instance, self.name, data)
return data
def __repr__(self):
return '<TestData: name=%r, data=%r>' % (self.name, self.data)
class TestCase(TransactionTestCase): class TestCase(TransactionTestCase):
""" """
Similar to TransactionTestCase, but use `transaction.atomic()` to achieve Similar to TransactionTestCase, but use `transaction.atomic()` to achieve
@ -1119,12 +1174,16 @@ class TestCase(TransactionTestCase):
cls._rollback_atomics(cls.cls_atomics) cls._rollback_atomics(cls.cls_atomics)
cls._remove_databases_failures() cls._remove_databases_failures()
raise raise
pre_attrs = cls.__dict__.copy()
try: try:
cls.setUpTestData() cls.setUpTestData()
except Exception: except Exception:
cls._rollback_atomics(cls.cls_atomics) cls._rollback_atomics(cls.cls_atomics)
cls._remove_databases_failures() cls._remove_databases_failures()
raise raise
for name, value in cls.__dict__.items():
if value is not pre_attrs.get(name):
setattr(cls, name, TestData(name, value))
@classmethod @classmethod
def tearDownClass(cls): def tearDownClass(cls):

View File

@ -15,6 +15,10 @@ about each item can often be found in the release notes of two versions prior.
See the :ref:`Django 3.2 release notes <deprecated-features-3.2>` for more See the :ref:`Django 3.2 release notes <deprecated-features-3.2>` for more
details on these changes. details on these changes.
* Support for assigning objects which don't support creating deep copies with
``copy.deepcopy()`` to class attributes in ``TestCase.setUpTestData()`` will
be removed.
.. _deprecation-removed-in-4.0: .. _deprecation-removed-in-4.0:
4.0 4.0

View File

@ -195,7 +195,10 @@ Templates
Tests Tests
~~~~~ ~~~~~
* ... * Objects assigned to class attributes in :meth:`.TestCase.setUpTestData` are
now isolated for each test method. Such objects are now required to support
creating deep copies with :py:func:`copy.deepcopy`. Assigning objects which
don't support ``deepcopy()`` is deprecated and will be removed in Django 4.1.
URLs URLs
~~~~ ~~~~
@ -255,4 +258,6 @@ Features deprecated in 3.2
Miscellaneous Miscellaneous
------------- -------------
* ... * Assigning objects which don't support creating deep copies with
:py:func:`copy.deepcopy` to class attributes in
:meth:`.TestCase.setUpTestData` is deprecated.

View File

@ -873,11 +873,13 @@ It also provides an additional method:
(for instance, MySQL with the MyISAM engine), ``setUpTestData()`` will be (for instance, MySQL with the MyISAM engine), ``setUpTestData()`` will be
called before each test, negating the speed benefits. called before each test, negating the speed benefits.
Be careful not to modify any objects created in ``setUpTestData()`` in .. versionchanged:: 3.2
your test methods. Modifications to in-memory objects from setup work done
at the class level will persist between test methods. If you do need to Objects assigned to class attributes in ``setUpTestData()`` must
modify them, you could reload them in the ``setUp()`` method with support creating deep copies with :py:func:`copy.deepcopy` in order to
:meth:`~django.db.models.Model.refresh_from_db`, for example. isolate them from alterations performed by each test methods. In
previous versions of Django these objects were reused and changes made
to them were persisted between test methods.
.. _live-test-server: .. _live-test-server:

View File

@ -12,4 +12,4 @@ class Person(models.Model):
class PossessedCar(models.Model): class PossessedCar(models.Model):
car = models.ForeignKey(Car, models.CASCADE) car = models.ForeignKey(Car, models.CASCADE)
belongs_to = models.ForeignKey(Person, models.CASCADE) belongs_to = models.ForeignKey(Person, models.CASCADE, related_name='possessed_cars')

View File

@ -1,7 +1,11 @@
from django.db import IntegrityError, connections, transaction from functools import wraps
from django.test import TestCase, skipUnlessDBFeature
from .models import Car, PossessedCar from django.db import IntegrityError, connections, transaction
from django.test import TestCase, ignore_warnings, skipUnlessDBFeature
from django.test.testcases import TestData
from django.utils.deprecation import RemovedInDjango41Warning
from .models import Car, Person, PossessedCar
class TestTestCase(TestCase): class TestTestCase(TestCase):
@ -38,3 +42,95 @@ class TestTestCase(TestCase):
) )
with self.assertRaisesMessage(AssertionError, message): with self.assertRaisesMessage(AssertionError, message):
Car.objects.using('other').get() Car.objects.using('other').get()
class NonDeepCopyAble:
def __deepcopy__(self, memo):
raise TypeError
def assert_no_queries(test):
@wraps(test)
def inner(self):
with self.assertNumQueries(0):
test(self)
return inner
class TestDataTests(TestCase):
# setUpTestData re-assignment are also wrapped in TestData.
jim_douglas = None
@classmethod
def setUpTestData(cls):
cls.jim_douglas = Person.objects.create(name='Jim Douglas')
cls.car = Car.objects.create(name='1963 Volkswagen Beetle')
cls.herbie = cls.jim_douglas.possessed_cars.create(
car=cls.car,
belongs_to=cls.jim_douglas,
)
cls.non_deepcopy_able = NonDeepCopyAble()
@assert_no_queries
def test_class_attribute_equality(self):
"""Class level test data is equal to instance level test data."""
self.assertEqual(self.jim_douglas, self.__class__.jim_douglas)
@assert_no_queries
def test_class_attribute_identity(self):
"""
Class level test data is not identical to instance level test data.
"""
self.assertIsNot(self.jim_douglas, self.__class__.jim_douglas)
@assert_no_queries
def test_identity_preservation(self):
"""Identity of test data is preserved between accesses."""
self.assertIs(self.jim_douglas, self.jim_douglas)
@assert_no_queries
def test_known_related_objects_identity_preservation(self):
"""Known related objects identity is preserved."""
self.assertIs(self.herbie.car, self.car)
self.assertIs(self.herbie.belongs_to, self.jim_douglas)
@ignore_warnings(category=RemovedInDjango41Warning)
def test_undeepcopyable(self):
self.assertIs(self.non_deepcopy_able, self.__class__.non_deepcopy_able)
def test_undeepcopyable_warning(self):
msg = (
"Assigning objects which don't support copy.deepcopy() during "
"setUpTestData() is deprecated. Either assign the "
"non_deepcopy_able attribute during setUpClass() or setUp(), or "
"add support for deepcopy() to "
"test_utils.test_testcase.TestDataTests.non_deepcopy_able."
)
with self.assertRaisesMessage(RemovedInDjango41Warning, msg):
self.non_deepcopy_able
def test_repr(self):
self.assertEqual(
repr(TestData('attr', 'value')),
"<TestData: name='attr', data='value'>",
)
class SetupTestDataIsolationTests(TestCase):
"""
In-memory data isolation is respected for model instances assigned to class
attributes during setUpTestData.
"""
@classmethod
def setUpTestData(cls):
cls.car = Car.objects.create(name='Volkswagen Beetle')
def test_book_name_deutsh(self):
self.assertEqual(self.car.name, 'Volkswagen Beetle')
self.car.name = 'VW sKäfer'
self.car.save()
def test_book_name_french(self):
self.assertEqual(self.car.name, 'Volkswagen Beetle')
self.car.name = 'Volkswagen Coccinelle'
self.car.save()