Code

Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#20680 closed Cleanup/optimization (fixed)

Deprecate django.utils.unittest

Reported by: aaugustin Owned by: aaugustin
Component: Utilities Version: master
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


Attachments (0)

Change History (10)

comment:1 Changed 10 months ago by aaugustin

  • Status changed from new to assigned
  • Summary changed from Deprecated django.utils.unittest to Deprecate django.utils.unittest

comment:2 Changed 10 months ago by Aymeric Augustin <aymeric.augustin@…>

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

In 7f264e02f4480c49d1440be882416a10951c2165:

Fixed #20680 -- Deprecated django.utils.unittest.

Refs #19204.

comment:3 Changed 10 months ago by Aymeric Augustin <aymeric.augustin@…>

In cfcf4b3605f9653e4e056088d89932b2a0e4281b:

Stopped using django.utils.unittest in the test suite.

Refs #20680.

comment:4 Changed 10 months ago by aaugustin

  • Resolution fixed deleted
  • Status changed from closed to new

There are two remaining instances of django.utils.unittest that cannot be replaced with unittest outright, because they import both unittest and django.utils.unittest. They're in django.test.simple and tests.test_discovery_sample.tests_sample, and they're related to the new test discovery introduced in Django 1.6.

comment:5 Changed 10 months ago by prestontimmons

If django.utils.unittest is gone, then TestUnittest2 can go away.

The vendored version caused problems for loadTestsFromName, so that we actually had to modify an issubclass check so discovery would work across the board:

https://github.com/django/django/commit/9012833af857e081b515ce760685b157638efcef#L43L128

I think that django.test.simple can be modified for the same reason. This issubclass check is now redundant:

https://github.com/django/django/blob/master/django/test/simple.py#L191

comment:6 Changed 10 months ago by carljm

The testcases in test_discovery_sample.tests_sample are for use by the test_runner.test_discover_runner tests, and they are verifying that test discovery (and naming specific testcases on the commandline) works whether you inherit from django.utils.unittest.TestCase, unittest.TestCase, or django.test.TestCase. At this point the first two cases should be the same unless you have unittest2 installed from PyPI, and it's not all that critical to keep testing the first case since it's deprecated anyway. If silencing the deprecation warning is a pain, I have no problem with just removing the django.utils.unittest sample (and its associated tests in test_discover_runner).

The usage of both unittest (as real_unittest) and django.utils.unittest in django.test.simple predates (and wasn't modified by) the test discovery patch (it dates back to #15368). The specific issubclass call where both are used is redundant, since unittest2.TestCase inherits from unittest.TestCase anyway. Switching the entire file to use plain unittest could cause a change in behavior in the case where unittest2 from PyPI is installed, since django.utils.unittest uses it in preference to stdlib unittest. I think the safest option is to just continue to import django.utils.unittest there; since django.test.simple is deprecated anyway, there's no harm in one deprecated module using another.

comment:7 Changed 10 months ago by aaugustin

Since silencing warnings at import time is painful, I'll remove the test cases that can be removed.

You have a point about django.test.simple, I'll just add a comment to explain why we keep using django.utils.unittest there.

comment:8 Changed 10 months ago by Aymeric Augustin <aymeric.augustin@…>

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

In 909433fa506dc3c8412cecb4439049acb9a3f447:

Removed tests for django.utils.unittest vs. unittest.

Silenced warnings caused by the deprecation of django.utils.unittest.

Thanks Preston Timmons and Carl Meyer for their advice.

Fixed #20680.

comment:9 Changed 10 months ago by Aymeric Augustin <aymeric.augustin@…>

In 09b446dfe86f01f9d21e8c6351a31c5587a5b7a9:

This doesn't need to be a package any more.

Refs #20680.

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

In e021b87c0009c11aa2e62b62fc40b4b0209a8e5d:

Fixed a few more imports of django.utils.unittest.

One import per line please! Refs #20680.

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.