Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#25860 closed Cleanup/optimization (fixed)

Document transaction leak possiblity in TestCase

Reported by: Jonas Haag Owned by: nobody
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Coming from https://groups.google.com/forum/#!topic/django-developers/t1xaSmNMuKQ, I found a bug in the test database setup. This is probably related to #24080 but I'm not 100% sure.

The error can be shown with a specific series of skipped and non-skipped tests. I attached a minimal example project that demonstrates the issue.

This used to work in Django 1.7, and doesn't work in any Django versions later than 1.7. It is caused, according to a Git bisect, by https://github.com/django/django/commit/da9fe5c717c179a9e881a40efd3bfe36a21bf4a6.

Attachments (1)

t.tar.bz2 (2.1 KB ) - added by Jonas Haag 8 years ago.
Minimal test project

Download all attachments as: .zip

Change History (8)

by Jonas Haag, 8 years ago

Attachment: t.tar.bz2 added

Minimal test project

comment:1 by Tim Graham, 8 years ago

If you raise SkipTest in setUpClass() after calling super(), then I'd think the database setup in TestCase.setUpClass() happens but not the teardown in TestCase.tearDownClass? If so, I'm not sure there's much Django can do about the issue. Any suggestions? Can you raise SkipTest before calling super()?

comment:2 by Carl Meyer, 8 years ago

Without digging into it more I don't know if this is something that would need to be fixed in Django or in stdlib unittest, but ISTM that ideally even if SkipTest is raised after setUpClass, the testing framework ought to be responsible to make sure that tearDownClass is still called regardless.

comment:3 by Jonas Haag, 8 years ago

I agree with carljm.

comment:4 by Jonas Haag, 8 years ago

From the Python docs:

If an exception is raised during a setUpClass then the tests in the class are not run and the tearDownClass is not run. Skipped classes will not have setUpClass or tearDownClass run. If the exception is a SkipTest exception then the class will be reported as having been skipped instead of as an error.

Looks like nothing's wrong with Django here.

comment:5 by Tim Graham, 8 years ago

Component: Testing frameworkDocumentation
Keywords: regression removed
Summary: Test setup error: Cannot operate on a closed databaseDocument transaction leak possiblity in TestCase
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

How about a documentation patch to raise awareness?

comment:6 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In a5619f7e:

Fixed #25860 -- Documented a transaction leak possiblity in TestCase.

Thanks Jonas Haag for report and review.

comment:7 by Tim Graham <timograham@…>, 8 years ago

In 35b5e11:

[1.9.x] Fixed #25860 -- Documented a transaction leak possiblity in TestCase.

Thanks Jonas Haag for report and review.

Backport of a5619f7ed3bc2ca07b428fa0abfaaff088378f48 from master

Note: See TracTickets for help on using tickets.
Back to Top