TestCase with multiple assertRaises fails with TransactionManagementError

[Kevin Christopher Henry]

This test case:

class NameModel(models.Model):
    first_name = models.CharField(unique=True, max_length=30)
    last_name = models.CharField(unique=True, max_length=30)

# passes in 1.5
# passes with TransactionTestCase
class UniqueTestCase(TestCase):

    def setUp(self):
        NameModel.objects.create(first_name="John", last_name="Doe")

    # passes with either, but not both, of these assertRaises()
    def test_unique(self):
        self.assertRaises(IntegrityError, NameModel.objects.create, first_name="John", last_name="Hancock")
        self.assertRaises(IntegrityError, NameModel.objects.create, first_name="A", last_name="Doe")

...fails with:

======================================================================
ERROR: test_unique (example.tests.UniqueTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Files\Documents\Programs\Trivia\assert_raises_bug\example\tests.py", line 16, in test_unique
    self.assertRaises(IntegrityError, NameModel.objects.create, first_name="A", last_name="Doe")
  File "C:\Program Other\Python27\Lib\unittest\case.py", line 475, in assertRaises
    callableObj(*args, **kwargs)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\manager.py", line 157, in create
    return self.get_queryset().create(**kwargs)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\query.py", line 319, in create
    obj.save(force_insert=True, using=self.db)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\base.py", line 545, in save
    force_update=force_update, update_fields=update_fields)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\base.py", line 573, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\base.py", line 654, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\base.py", line 687, in _do_insert
    using=using, raw=raw)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\manager.py", line 232, in _insert
    return insert_query(self.model, objs, fields, **kwargs)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\query.py", line 1511, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\models\sql\compiler.py", line 898, in execute_sql
    cursor.execute(sql, params)
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\backends\util.py", line 47, in execute
    self.db.validate_no_broken_transaction()
  File "C:\PythonEnv\Trivia\lib\site-packages\django\db\backends\__init__.py", line 365, in validate_no_broken_transaction
    "An error occurred in the current transaction. You can't "
TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.

I assume the problem here is that the TestCase method is being run as a transaction, but the individual assertRaises() are not themselves using atomic transactions to run the passed-in code.

[Aymeric Augustin]

The correct code is:

    def test_unique(self):
        with self.assertRaises(IntegrityError):
            # roll back transaction to a clean state before letting the IntegrityError bubble up
            with transaction.atomic():
                NameModel.objects.create(first_name="John", last_name="Hancock")
        with self.assertRaises(IntegrityError):
            # roll back transaction to a clean state before letting the IntegrityError bubble up
            with transaction.atomic():
                NameModel.objects.create(first_name="A", last_name="Doe")

This is explained in the documentation of the new transaction management in Django 1.6. You're seeing the expected (if not totally user-friendly) behavior.

In Django 1.5, this would not blow up in tests, but it would blow up in actual code. I understand that you find this error annoying, but it least you get the same behavior in tests and in actual code in Django 1.6.

I've spent lot of time thinking about this behavior and it's a Really Hard Problem (at least for me). If you have concrete suggestions, let's discuss on the django-developers mailing list. You may want to check past discussions first, as well as the videos of ​my talks at DjangoCon Europe and DjangoCon US 2013 for more background on the design.

I don't think it's a good idea to wrap the body of an assertRaises block in a transaction automatically as it may be used for many purposes that do not involve transactions.

[Kevin Christopher Henry]

To be clear, I'm not criticizing the design of the transaction management system. I understand why database exceptions need to be caught outside of an atomic block.

What worries me here is that a user can have (non-transactional) code that is perfectly correct that nonetheless produces test errors simply because of the implementation details of TestCase (namely that it uses transactions instead of table truncation). It's not just assertRaises() - any piece of code that, say, catches a database error and then continues (without using transactions) will result in a test error even though it's correct.

Of course, if the user understands transaction management they can figure out this error and work around it in the manner you describe. But transaction management is an "advanced" feature (per the ​documentation) and users shouldn't be forced to learn it to get their tests to properly validate their working, non-transactional code. Practically speaking, many won't, leading to spurious and confusing errors like the one above.

The right answer, I think, is that such cases require TransactionTestCase. My proposal would be to update the documentation about when to use TransactionTestCase vs. TestCase. Right now it ​reads: "If your test requires testing of such transactional behavior, you should use a Django TransactionTestCase. TransactionTestCase and TestCase are identical except for... the ability for test code to test the effects of commit and rollback.... Always use TransactionTestCase when testing transactional behavior." This is too limited, I think it needs to be broadened to suggest TransactionTestCase for any (non-transactional) code that catches database errors.

I'm not sure of the best wording for this, but I'd be happy to take a stab at it if there's agreement that this is a good idea.

[Aymeric Augustin]

The difficulties are:

• TransactionTestCase is very inefficient when you have many models (starting at a few dozens).
• The problem is not limited to database errors, it is known to happen if a post-save signal handler raises an exception. That makes it hard to be provide clear instructions.

[Aymeric Augustin]

I'm bumping this ticket back to unreviewed, given that you've reframed it as a documentation issue.

[Kevin Christopher Henry]

Discussion here: ​https://groups.google.com/forum/#!topic/django-developers/26ZCcUPubro.

[Aymeric Augustin]

Accepting on the basis that we should at least document the intended pattern for this particular use case:

def test_error():
    with self.assertRaises(IntegrityError):
        with transaction.atomic():
            raise_integrity_error()

[Chris Jerdonek]

I also just ran into this issue and agree improvements to the documentation would help. A couple suggestions as someone new to Django and using 1.6:

When I saw this error in my test:

TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.

I immediately read the Django docs about ​database transactions and my immediate thoughts were, "that's strange, I'm not doing any manual transaction management and I haven't touched the Django default to use autocommit."

My suggestion then would be to say something about TestCase vs TransactionTestCase in the main "Database transactions" docs (including a warning even), and not just include this in the testing docs. The reason is that while the main docs say Django defaults to autocommit, "autocommit" doesn't really seem to describe the behavior while testing using TestCase. Hence, the error behavior while testing is unexpected.

I also agree that the current wording in the testing section could be more explicit. Right now, the focus of the wording is on the explicit testing of transactions, whereas even just implicitly relying on rollback, etc. makes you affected.

[Chris Jerdonek]

I recently opened a new ticket #21836 and submitted a pull request for it. That pull request is related to this issue because I think it will make the TransactionManagementError described in this issue a little less surprising. This is because the pull request adds a sentence to the main transaction docs saying that TestCase wraps its tests in transactions. Currently, this fact is mentioned only in the testing docs and not in the main transaction docs.

[Shai Berger]

I'm going to try to add something here

[sheenarbw]

I just checked, this bug is still a bug as of commit d62563cbb194c420f242bfced52b37d6638e67c6