Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#12991 closed (fixed)

Django 1.2 should have optional support for unittest2

Reported by: ericholscher Owned by: russellm
Component: Testing framework Version: master
Severity: Keywords:
Cc: kmike, sean@…, bugs@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The new additions for unittest2 are awesome, and we should be using them!

I don't know if we want to ship with unittest2 in django, or just do a conditional import in the Test Suite, but I think that 1.2 should support the ability to use the new fancy features in unittest.

Attachments (7)

unittest2_docs_patch.diff (3.0 KB) - added by PaulM 4 years ago.
Adds information about unittest changes to contributing and testing docs.
unittest2-addition.diff (124.8 KB) - added by PaulM 4 years ago.
patch to bundle unittest2, fix failing tests, and use system version if possible. Updated to work against trunk.
t12991-alpha1.diff.gz (95.9 KB) - added by russellm 4 years ago.
First draft of unittest2 support for trunk
t12991-alpha2.diff.gz (56.5 KB) - added by russellm 4 years ago.
Second draft of adding unittest2 to trunk
t12991-rc1.diff.gz (57.3 KB) - added by russellm 4 years ago.
RC1 of patch to add unittest2 support
t12991-rc2.diff.gz (58.1 KB) - added by russellm 4 years ago.
RC2 of unittest2 patch
t12991-rc2__unittest_catchbreak.diff (7.4 KB) - added by lrekucki 4 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 4 years ago by ericholscher

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

We should also think about trying to move toward the new unit test discovery framework instead of rolling our own.

I talked to Michael Foord, and he says that doing:

try:

import unittest2 as unittest

except ImportError:

import unittest

Should get us some nice new features, like the fancy dict diffing, for free. Then we can use that as a stepping ground for getting more of the useful stuff in core.

comment:2 Changed 4 years ago by russellm

  • milestone 1.2 deleted
  • Triage Stage changed from Unreviewed to Accepted
  • Version SVN deleted

This is worth pursing, but would be a significant feature addition (or at least feature change), so it will need to wait for a later release.

Exactly how we can use it will require some discussion - while it would be great to switch to using unittest2's test discovery, our usual policy of not requiring dependencies will mean we need to continue to ship a native unittest version (or ship a copy of unittest2).

comment:3 Changed 4 years ago by voidspace

If there is anything I can do to support Django in using unittest2 let me know. Not just test discovery - the new addCleanups, new asserts, conditional skips and expected failures could be very useful for Django.

If you just have optional support for unittest2 (as this ticket suggests) then you still get the better failure messages for assertEqual when unittest2 is available.

comment:4 Changed 4 years ago by voidspace

unittest2 now has an implementation of failfast and control-c handling, which may duplicate some of your test runner functionality. If you *ship* unittest2 it may mean you can delete some code from your test runner. There are also class / module level fixtures (setUpClass etc).

comment:5 Changed 4 years ago by anonymous

  • Cc kmike added

comment:6 Changed 4 years ago by PaulM

  • Has patch set
  • Owner changed from nobody to PaulM
  • Status changed from new to assigned

Attached a patch which bundles unittest2 as discussed in -dev and with the core devs.

comment:7 Changed 4 years ago by PaulM

Just an update to note that this patch passes for python 2.4, 2.5, and 2.6 on my systems with sqlite3 and postgres 8.4 and 8.3 (when #13821 is patched).

comment:8 Changed 4 years ago by ericholscher

  • milestone set to 1.3
  • Needs documentation set
  • Version set to SVN

Paul:

We probably need to add documentation for these changes. I'm thinking in the Testing Django document, and maybe a note in the contributing section as well. Otherwise this looks good to me.

A summary of the patch:

The changes in tests/regressiontests/test_client_regress/models.py are to address changes in the string output of exceptions in unittest2. Otherwise this is just the importing of stock unittest2 with 2 changes:

  • The imports inside of unittest2 where absolute (from unittest2...) -- we had to change these to import from the django source tree so they are valid. This is the same approach taken in the simplejson module already included in Django.
  • The init.py was moved to another file and the new init is doing some magic to check if the user already has 2.7 or another version of unittest2 installed, and uses that before the bundled one. This is because it is assumed that people might have more up to date versions of the package than we ship.

The import for the new package is django.utils.unittest -- we made this unittest instead of unittest2 because unittest2 is actually just the unittest package from 2.7. So you can import django.utils.unittest in your package and know you are getting the 2.7 version of unittest.

comment:9 Changed 4 years ago by PaulM

One possible backwards compatibility issue this brings up is in user tests. Because unittest2 changes error reporting to be more verbose, users who are doing the wrong thing and have written tests which compare against exact, specific error strings (as was done in test_client_regress/models.py), there is a possibility that some user unittests may not work after this patch. This problem is introduced at the python level, and it seems that the general Django policy is not to support users who are doing the wrong thing.

comment:10 Changed 4 years ago by zimnyx

Please take look on ticket #14049: Fixture loading should be skipped for TestCase decorated with @skip*

Changed 4 years ago by PaulM

Adds information about unittest changes to contributing and testing docs.

comment:11 Changed 4 years ago by PaulM

  • Needs documentation unset

I think the docs patch resolves ericholscher's concerns. I will address #14049 in a separate patch related to that issue, so as not to complicate this change further.

Changed 4 years ago by PaulM

patch to bundle unittest2, fix failing tests, and use system version if possible. Updated to work against trunk.

comment:12 Changed 4 years ago by PaulM

Updated the patch to apply cleanly against trunk as of #13567.

comment:13 Changed 4 years ago by seanc

  • Cc sean@… added

comment:14 Changed 4 years ago by anonymous

  • Cc bugs@… added

Changed 4 years ago by russellm

First draft of unittest2 support for trunk

comment:15 Changed 4 years ago by russellm

  • Owner changed from PaulM to russellm
  • Status changed from assigned to new

Second draft patch now available for testing. See the mailing list thread for details on the changes. Any assistance testing this patch on multiple python versions, database versions, and especially on Oracle and GeoDjango would be gratefully accepted.

Changed 4 years ago by russellm

Second draft of adding unittest2 to trunk

Changed 4 years ago by russellm

RC1 of patch to add unittest2 support

comment:16 Changed 4 years ago by lukeplant

I just applied this and ran the test suite with Python 2.4/SQLite (with the patch on #14406 applied), and it ran with no failures (16 tests skipped).

Changed 4 years ago by russellm

RC2 of unittest2 patch

comment:17 Changed 4 years ago by lrekucki

Run this on Python 2.4 with SQLite and PostgreSQL 8.4 (that took a while). All skips look reasonable. Some small things:

  • the keyboard_interrupt code isn't needed anymore - unittest2 can do that already. Same thing for the failfast code, so DjangoTestRunner is a noop now.
  • I remebered AdminDocsTest needed to be skipped without doctools, so I changed it to use skipUnless like other docutils depended test.

Patch against your patch attached.

Changed 4 years ago by lrekucki

comment:18 Changed 4 years ago by russellm

  • Triage Stage changed from Accepted to Ready for checkin

Thanks Łukasz; I've integrated those changes into the patch I'll commit tonight.

comment:19 Changed 4 years ago by russellm

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

(In [14139]) Fixed #12991 -- Added unittest2 support. Thanks to PaulM for the draft patch, and to Luke, Karen, Justin, Alex, Łukasz Rekucki, and Chuck Harmston for their help testing and reviewing the final patch.

comment:20 Changed 4 years ago by russellm

(In [14140]) Refs #12991 -- Added extra docs for the unittest2 changes made in r14139.

comment:21 Changed 4 years ago by russellm

(In [14186]) Refs #12991 -- Added release note about deprecation of DjangoTestRunner.

comment:22 Changed 4 years ago by voidspace

The current import logic for unittest/unittest2 is:

If Python 2.7 use standard library unittest
Try locally installed unittest2
Otherwise fallback to bundled unittest2

Can I suggest changing it to:

Try locally installed unittest2
If Python 2.7 use standard library unittest
Otherwise fallback

This allows Python 2.7 users to still upgrade to a more recent version of unittest2 and use that locally. (New features added to unittest in Python 3.2 are still being added to unittest2.) Without this Python 2.6 (and earlier) users will be able to use a locally installed unittest2 but Python 2.7 users *won't*, which seems inconsistent.

comment:23 Changed 4 years ago by voidspace

I've created a new ticket for my last comment: issue 14486.

comment:24 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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.