Permalink
Browse files

Fixed #18271 -- Changed stage at which TransactionTestCase flushes DB…

… tables.

Previously, the flush was done before the test case execution and now
it is performed after it.

Other changes to the testing infrastructure include:

* TransactionTestCase now doesn't reset autoincrement sequences either
  (previous behavior can achieved by using `reset_sequences`.)
  With this, no implicit such reset is performed by any of the provided
  TestCase classes.

* New ordering of test cases: All unittest tes cases are run first and
  doctests are run at the end.

THse changes could be backward-incompatible with test cases that relied
on some kind of state being preserved between tests. Please read the
relevant sections of the release notes and testing documentation for
further details.

Thanks Andreas Pelme for the initial patch. Karen Tracey and Anssi
Kääriäinen for the feedback and Anssi for reviewing.

This also fixes #12408.
  • Loading branch information...
1 parent 38ce709 commit f758bdab5eec3e615598948dd5bcf9bb7b910c9d @ramiro ramiro committed Jul 24, 2012
@@ -29,6 +29,8 @@ def handle_noargs(self, **options):
connection = connections[db]
verbosity = int(options.get('verbosity'))
interactive = options.get('interactive')
+ # 'reset_sequences' is a stealth option
+ reset_sequences = options.get('reset_sequences', True)
self.style = no_style()
@@ -40,7 +42,7 @@ def handle_noargs(self, **options):
except ImportError:
pass
- sql_list = sql_flush(self.style, connection, only_django=True)
+ sql_list = sql_flush(self.style, connection, only_django=True, reset_sequences=reset_sequences)
if interactive:
confirm = raw_input("""You have requested a flush of the database.
@@ -98,7 +98,7 @@ def sql_delete(app, style, connection):
return output[::-1] # Reverse it, to deal with table dependencies.
-def sql_flush(style, connection, only_django=False):
+def sql_flush(style, connection, only_django=False, reset_sequences=True):
"""
Returns a list of the SQL statements used to flush the database.
@@ -109,9 +109,8 @@ def sql_flush(style, connection, only_django=False):
tables = connection.introspection.django_table_names(only_existing=True)
else:
tables = connection.introspection.table_names()
- statements = connection.ops.sql_flush(
- style, tables, connection.introspection.sequence_list()
- )
+ seqs = connection.introspection.sequence_list() if reset_sequences else ()
+ statements = connection.ops.sql_flush(style, tables, seqs)
return statements
def sql_custom(app, style, connection):
@@ -748,11 +748,24 @@ def sql_flush(self, style, tables, sequences):
the given database tables (without actually removing the tables
themselves).
+ The returned value also includes SQL statements required to reset DB
+ sequences passed in :param sequences:.
+
The `style` argument is a Style object as returned by either
color_style() or no_style() in django.core.management.color.
"""
raise NotImplementedError()
+ def sequence_reset_by_name_sql(self, style, sequences):
+ """
+ Returns a list of the SQL statements required to reset sequences
+ passed in :param sequences:.
+
+ The `style` argument is a Style object as returned by either
+ color_style() or no_style() in django.core.management.color.
+ """
+ return []
+
def sequence_reset_sql(self, style, model_list):
"""
Returns a list of the SQL statements required to reset sequences for
@@ -262,22 +262,25 @@ def sql_flush(self, style, tables, sequences):
for table in tables:
sql.append('%s %s;' % (style.SQL_KEYWORD('TRUNCATE'), style.SQL_FIELD(self.quote_name(table))))
sql.append('SET FOREIGN_KEY_CHECKS = 1;')
-
- # Truncate already resets the AUTO_INCREMENT field from
- # MySQL version 5.0.13 onwards. Refs #16961.
- if self.connection.mysql_version < (5,0,13):
- sql.extend(
- ["%s %s %s %s %s;" % \
- (style.SQL_KEYWORD('ALTER'),
- style.SQL_KEYWORD('TABLE'),
- style.SQL_TABLE(self.quote_name(sequence['table'])),
- style.SQL_KEYWORD('AUTO_INCREMENT'),
- style.SQL_FIELD('= 1'),
- ) for sequence in sequences])
+ sql.extend(self.sequence_reset_by_name_sql(style, sequences))
return sql
else:
return []
+ def sequence_reset_by_name_sql(self, style, sequences):
+ # Truncate already resets the AUTO_INCREMENT field from
+ # MySQL version 5.0.13 onwards. Refs #16961.
+ if self.connection.mysql_version < (5, 0, 13):
+ return ["%s %s %s %s %s;" % \
+ (style.SQL_KEYWORD('ALTER'),
+ style.SQL_KEYWORD('TABLE'),
+ style.SQL_TABLE(self.quote_name(sequence['table'])),
+ style.SQL_KEYWORD('AUTO_INCREMENT'),
+ style.SQL_FIELD('= 1'),
+ ) for sequence in sequences]
+ else:
+ return []
+
def validate_autopk_value(self, value):
# MySQLism: zero in AUTO_INCREMENT field does not work. Refs #17653.
if value == 0:
@@ -298,18 +298,23 @@ def sql_flush(self, style, tables, sequences):
for table in tables]
# Since we've just deleted all the rows, running our sequence
# ALTER code will reset the sequence to 0.
- for sequence_info in sequences:
- sequence_name = self._get_sequence_name(sequence_info['table'])
- table_name = self.quote_name(sequence_info['table'])
- column_name = self.quote_name(sequence_info['column'] or 'id')
- query = _get_sequence_reset_sql() % {'sequence': sequence_name,
- 'table': table_name,
- 'column': column_name}
- sql.append(query)
+ sql.extend(self.sequence_reset_by_name_sql(style, sequences))
return sql
else:
return []
+ def sequence_reset_by_name_sql(self, style, sequences):
+ sql = []
+ for sequence_info in sequences:
+ sequence_name = self._get_sequence_name(sequence_info['table'])
+ table_name = self.quote_name(sequence_info['table'])
+ column_name = self.quote_name(sequence_info['column'] or 'id')
+ query = _get_sequence_reset_sql() % {'sequence': sequence_name,
+ 'table': table_name,
+ 'column': column_name}
+ sql.append(query)
+ return sql
+
def sequence_reset_sql(self, style, model_list):
from django.db import models
output = []
@@ -85,25 +85,29 @@ def sql_flush(self, style, tables, sequences):
(style.SQL_KEYWORD('TRUNCATE'),
style.SQL_FIELD(', '.join([self.quote_name(table) for table in tables]))
)]
-
- # 'ALTER SEQUENCE sequence_name RESTART WITH 1;'... style SQL statements
- # to reset sequence indices
- for sequence_info in sequences:
- table_name = sequence_info['table']
- column_name = sequence_info['column']
- if not (column_name and len(column_name) > 0):
- # This will be the case if it's an m2m using an autogenerated
- # intermediate table (see BaseDatabaseIntrospection.sequence_list)
- column_name = 'id'
- sql.append("%s setval(pg_get_serial_sequence('%s','%s'), 1, false);" % \
- (style.SQL_KEYWORD('SELECT'),
- style.SQL_TABLE(self.quote_name(table_name)),
- style.SQL_FIELD(column_name))
- )
+ sql.extend(self.sequence_reset_by_name_sql(style, sequences))
return sql
else:
return []
+ def sequence_reset_by_name_sql(self, style, sequences):
+ # 'ALTER SEQUENCE sequence_name RESTART WITH 1;'... style SQL statements
+ # to reset sequence indices
+ sql = []
+ for sequence_info in sequences:
+ table_name = sequence_info['table']
+ column_name = sequence_info['column']
+ if not (column_name and len(column_name) > 0):
+ # This will be the case if it's an m2m using an autogenerated
+ # intermediate table (see BaseDatabaseIntrospection.sequence_list)
+ column_name = 'id'
+ sql.append("%s setval(pg_get_serial_sequence('%s','%s'), 1, false);" % \
+ (style.SQL_KEYWORD('SELECT'),
+ style.SQL_TABLE(self.quote_name(table_name)),
+ style.SQL_FIELD(column_name))
+ )
+ return sql
+
def tablespace_sql(self, tablespace, inline=False):
if inline:
return "USING INDEX TABLESPACE %s" % self.quote_name(tablespace)
@@ -5,7 +5,7 @@
from django.db.models import get_app, get_apps
from django.test import _doctest as doctest
from django.test.utils import setup_test_environment, teardown_test_environment
-from django.test.testcases import OutputChecker, DocTestRunner, TestCase
+from django.test.testcases import OutputChecker, DocTestRunner
from django.utils import unittest
from django.utils.importlib import import_module
from django.utils.module_loading import module_has_submodule
@@ -263,7 +263,7 @@ def build_suite(self, test_labels, extra_tests=None, **kwargs):
for test in extra_tests:
suite.addTest(test)
- return reorder_suite(suite, (TestCase,))
+ return reorder_suite(suite, (unittest.TestCase,))
def setup_databases(self, **kwargs):
from django.db import connections, DEFAULT_DB_ALIAS
@@ -23,6 +23,7 @@
from django.core.exceptions import ValidationError, ImproperlyConfigured
from django.core.handlers.wsgi import WSGIHandler
from django.core.management import call_command
+from django.core.management.color import no_style
from django.core.signals import request_started
from django.core.servers.basehttp import (WSGIRequestHandler, WSGIServer,
WSGIServerException)
@@ -444,10 +445,15 @@ def assertHTMLNotEqual(self, html1, html2, msg=None):
class TransactionTestCase(SimpleTestCase):
+
# The class we'll use for the test client self.client.
# Can be overridden in derived classes.
client_class = Client
+ # Subclasses can ask for resetting of auto increment sequence before each
+ # test case
+ reset_sequences = False
+
def _pre_setup(self):
"""Performs any pre-test setup. This includes:
@@ -462,22 +468,36 @@ def _pre_setup(self):
self._urlconf_setup()
mail.outbox = []
+ def _reset_sequences(self, db_name):
+ conn = connections[db_name]
+ if conn.features.supports_sequence_reset:
+ sql_list = \
+ conn.ops.sequence_reset_by_name_sql(no_style(),
+ conn.introspection.sequence_list())
+ if sql_list:
+ try:
+ cursor = conn.cursor()
+ for sql in sql_list:
+ cursor.execute(sql)
+ except Exception:
+ transaction.rollback_unless_managed(using=db_name)
+ raise
+ transaction.commit_unless_managed(using=db_name)
+
def _fixture_setup(self):
- # If the test case has a multi_db=True flag, flush all databases.
- # Otherwise, just flush default.
- if getattr(self, 'multi_db', False):
- databases = connections
- else:
- databases = [DEFAULT_DB_ALIAS]
- for db in databases:
- call_command('flush', verbosity=0, interactive=False, database=db,
- skip_validation=True)
+ # If the test case has a multi_db=True flag, act on all databases.
+ # Otherwise, just on the default DB.
+ db_names = connections if getattr(self, 'multi_db', False) else [DEFAULT_DB_ALIAS]
+ for db_name in db_names:
+ # Reset sequences
+ if self.reset_sequences:
+ self._reset_sequences(db_name)
if hasattr(self, 'fixtures'):
# We have to use this slightly awkward syntax due to the fact
# that we're using *args and **kwargs together.
call_command('loaddata', *self.fixtures,
- **{'verbosity': 0, 'database': db, 'skip_validation': True})
+ **{'verbosity': 0, 'database': db_name, 'skip_validation': True})
def _urlconf_setup(self):
if hasattr(self, 'urls'):
@@ -534,7 +554,12 @@ def _post_teardown(self):
conn.close()
def _fixture_teardown(self):
- pass
+ # If the test case has a multi_db=True flag, flush all databases.
+ # Otherwise, just flush default.
+ databases = connections if getattr(self, 'multi_db', False) else [DEFAULT_DB_ALIAS]
+ for db in databases:
+ call_command('flush', verbosity=0, interactive=False, database=db,
+ skip_validation=True, reset_sequences=False)
def _urlconf_teardown(self):
if hasattr(self, '_old_root_urlconf'):
@@ -808,22 +833,21 @@ def _fixture_setup(self):
if not connections_support_transactions():
return super(TestCase, self)._fixture_setup()
+ assert not self.reset_sequences, 'reset_sequences cannot be used on TestCase instances'
+
# If the test case has a multi_db=True flag, setup all databases.
# Otherwise, just use default.
- if getattr(self, 'multi_db', False):
- databases = connections
- else:
- databases = [DEFAULT_DB_ALIAS]
+ db_names = connections if getattr(self, 'multi_db', False) else [DEFAULT_DB_ALIAS]
- for db in databases:
- transaction.enter_transaction_management(using=db)
- transaction.managed(True, using=db)
+ for db_name in db_names:
+ transaction.enter_transaction_management(using=db_name)
+ transaction.managed(True, using=db_name)
disable_transaction_methods()
from django.contrib.sites.models import Site
Site.objects.clear_cache()
- for db in databases:
+ for db in db_names:
if hasattr(self, 'fixtures'):
call_command('loaddata', *self.fixtures,
**{
View
@@ -188,6 +188,57 @@ Session not saved on 500 responses
Django's session middleware will skip saving the session data if the
response's status code is 500.
+Changes in tests execution
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Some changes have been introduced in the execution of tests that might be
+backward-incompatible for some testing setups:
+
+Database flushing in ``django.test.TransactionTestCase``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Previously, the test database was truncated *before* each test run in a
+:class:`~django.test.TransactionTestCase`.
+
+In order to be able to run unit tests in any order and to make sure they are
+always isolated from each other, :class:`~django.test.TransactionTestCase` will
+now reset the database *after* each test run instead.
+
+No more implict DB sequences reset
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+:class:`~django.test.TransactionTestCase` tests used to reset primary key
+sequences automatically together with the database flushing actions described
+above.
+
+This has been changed so no sequences are implicitly reset. This can cause
+:class:`~django.test.TransactionTestCase` tests that depend on hard-coded
+primary key values to break.
+
+The new :attr:`~django.test.TransactionTestCase.reset_sequences` attribute can
+be used to force the old behavior for :class:`~django.test.TransactionTestCase`
+that might need it.
+
+Ordering of tests
+^^^^^^^^^^^^^^^^^
+
+In order to make sure all ``TestCase`` code starts with a clean database,
+tests are now executed in the following order:
+
+* First, all unittests (including :class:`unittest.TestCase`,
+ :class:`~django.test.SimpleTestCase`, :class:`~django.test.TestCase` and
+ :class:`~django.test.TransactionTestCase`) are run with no particular ordering
+ guaranteed nor enforced among them.
+
+* Then any other tests (e.g. doctests) that may alter the database without
+ restoring it to its original state are run.
+
+This should not cause any problems unless you have existing doctests which
+assume a :class:`~django.test.TransactionTestCase` executed earlier left some
+database state behind or unit tests that rely on some form of state being
+preserved after the execution of other tests. Such tests are already very
+fragile, and must now be changed to be able to run independently.
+
Miscellaneous
~~~~~~~~~~~~~
Oops, something went wrong. Retry.

0 comments on commit f758bda

Please sign in to comment.