Code

Opened 16 months ago

Closed 14 months ago

Last modified 14 months ago

#20142 closed Bug (fixed)

Transaction methods are not re-enabled when fixture loading fails

Reported by: mlarente Owned by: filias
Component: Testing framework Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

When a test case fails to properly load a fixture (integrity error or some other reason), the transaction methods (which are disabled before the test is run) are not re-enabled before the next test is run. This can cause other tests (especially transaction test cases) to fail.

I've created a simple django application to reproduce this. You just have to add the application to a Django project, and then run:

manage.py test myapp.Before myapp.Broken myapp.After

This command is used to enforce the test order that shows the bug. The Before and After tests are actually the same test that checks if transaction methods are enabled (as they should be for a transaction test case). The first one passes and the other one fails:

import django.db.transaction
from django.test.testcases import TestCase
from django.test.testcases import TransactionTestCase

ORIGINAL = django.db.transaction.enter_transaction_management


class TransactionCheckMixin(object):
    def test_transaction_method(self):
        self.assertEqual(ORIGINAL, django.db.transaction.enter_transaction_management)


class Before(TransactionCheckMixin, TransactionTestCase):
    pass


class Broken(TestCase):
    fixtures = ['broken_fixture']

    def test_something(self):
        self.assertTrue(True)  # will not happen


class After(TransactionCheckMixin, TransactionTestCase):
    pass

The problem lies in the _fixture_setup method of the TestCase class:

class TestCase(TransactionTestCase):
    """
    Does basically the same as TransactionTestCase, but surrounds every test
    with a transaction, monkey-patches the real transaction management routines
    to do nothing, and rollsback the test transaction at the end of the test.
    You have to use TransactionTestCase, if you need transaction management
    inside a test.
    """

    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'

        for db_name in self._databases_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 self._databases_names(include_mirrors=False):
            if hasattr(self, 'fixtures'):
                call_command('loaddata', *self.fixtures,
                             **{
                                'verbosity': 0,
                                'commit': False,
                                'database': db,
                                'skip_validation': True,
                             })

It should handle errors and call the _fixture_teardown method if there's an error.

Attachments (1)

transaction-method-test.zip (1.7 KB) - added by mlarente 16 months ago.
Simple app that show the bug

Download all attachments as: .zip

Change History (5)

Changed 16 months ago by mlarente

Simple app that show the bug

comment:1 Changed 14 months ago by filias

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to filias
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

Confirmed the bug in django 1.5 and development version.

Last edited 14 months ago by filias (previous) (diff)

comment:2 Changed 14 months ago by filias

  • Easy pickings set
  • Has patch set

https://github.com/django/django/pull/1127

This fix does not contain tests as it is impossible to test it without mocking and a decision about using mocks in the project as yet not been made. This was discussed and agreed with Russel.

comment:3 Changed 14 months ago by Filipa Andrade <filipa.andrade@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 5883ae56b3e993052c46f7d9e756465b1387dd34:

Fixed #20142 -- Added error handling for fixture setup

TestCase._fixture_setup disables transactions so,
in case of an error, cleanup is needed to re-enable
transactions, otherwise following TransactionTestCase
would fail.

comment:4 Changed 14 months ago by Aymeric Augustin <aymeric.augustin@…>

In 5915800debef7cec01672dc0b467b7c2bd3aba03:

Merge pull request #1127 from filias/20142

Fixed #20142 -- Added error handling for fixture setup

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.