Skip to content

Fixed #21977 -- Deprecated SimpleTestCase.urls in favour of override_settings #2517

Closed
wants to merge 1 commit into from

3 participants

@coder9042

Replaced SimpleTestCase.urls with @override_settings(...).
Added warning in the function.
Updated the docs.

@coder9042

Updated PR as suggested on IRC.
Added test.

Thanks @loic for help.

@loic loic commented on an outdated diff Apr 5, 2014
tests/deprecation/tests.py
+ """
+ class TempTestCase(SimpleTestCase):
+ urls = 'tests.urls'
+
+ def test(self):
+ pass
+
+ with warnings.catch_warnings(record=True) as recorded:
+ suite = unittest.TestLoader().loadTestsFromTestCase(TempTestCase)
+ with open(os.devnull, 'w') as devnull:
+ unittest.TextTestRunner(stream=devnull, verbosity=2).run(suite)
+ msg = str(recorded.pop().message)
+ self.assertEqual(msg,
+ "SimpleTestCase.urls is deprecated and will be removed in"
+ "Django 2.0. Use @override_settings(ROOT_URLCONF=...)"
+ "in TempTestCase instead.")
@loic
Django member
loic added a note Apr 5, 2014

Missing blank line at the end of the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@loic loic commented on an outdated diff Apr 5, 2014
tests/deprecation/tests.py
+
+ def test_deprecation(self):
+ """
+ Ensure the correct warning is raised when SimpleTestCase.urls is used.
+ """
+ class TempTestCase(SimpleTestCase):
+ urls = 'tests.urls'
+
+ def test(self):
+ pass
+
+ with warnings.catch_warnings(record=True) as recorded:
+ suite = unittest.TestLoader().loadTestsFromTestCase(TempTestCase)
+ with open(os.devnull, 'w') as devnull:
+ unittest.TextTestRunner(stream=devnull, verbosity=2).run(suite)
+ msg = str(recorded.pop().message)
@loic
Django member
loic added a note Apr 5, 2014

I'd use force_text() instead, it's safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Apr 6, 2014
docs/internals/deprecation.txt
@@ -20,6 +20,9 @@ about each item can often be found in the release notes of two versions prior.
* Support for the ``prefix`` argument to
``django.conf.urls.i18n.i18n_patterns()`` will be removed.
+* ``SimpleTestCase.urls`` will be removed in favour of
@timgraham
Django member
timgraham added a note Apr 6, 2014

I think you can remove "in favor of ...." since we usually don't include the upgrade notes in this timeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Apr 6, 2014
docs/releases/1.8.txt
@@ -256,6 +256,14 @@ Updating your code is as simple as ensuring that ``urlpatterns`` is a list of
url('^other/$', views.otherview),
]
+``django.test.SimpleTestCase.urls``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The attribute :attr:`SimpleTestCase.urls <django.test.SimpleTestCase.urls>`
+for specifying URLconf configuration in tests has now been deprecated in
+favour of :func:`~django.test.override_settings`. The attribute will be removed
@timgraham
Django member
timgraham added a note Apr 6, 2014

favour->favor
mention override_settings(ROOT_URLCONF=...) here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Apr 6, 2014
docs/topics/testing/tools.txt
@@ -961,6 +961,10 @@ URLconf configuration
.. attribute:: SimpleTestCase.urls
+.. deprecated:: 1.8
+
+ Use :func:`~django.test.override_settings` for URLconf configuration.
@timgraham
Django member
timgraham added a note Apr 6, 2014

override_settings(ROOT_URLCONF=...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham and 1 other commented on an outdated diff Apr 6, 2014
tests/urlpatterns_reverse/tests.py
def test_404_tried_urls_have_names(self):
"""
Verifies that the list of URLs that come back from a Resolver404
exception contains a list in the right format for printing out in
the DEBUG 404 page with both the patterns and URL names, if available.
"""
- urls = 'urlpatterns_reverse.named_urls'
@timgraham
Django member
timgraham added a note Apr 6, 2014

I think this is just a local variable and not using the deprecation behavior in which case we don't need to change it, correct?

@coder9042
coder9042 added a note Apr 6, 2014

Yes. Changes reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Django member
$ flake8
./tests/deprecation/tests.py:227:1: E302 expected 2 blank lines, found 1
./tests/deprecation/tests.py:242:80: W291 trailing whitespace
@coder9042

@timgraham
PR updated.
Any other changes required?

@timgraham timgraham commented on the diff Apr 6, 2014
django/test/testcases.py
@@ -202,6 +204,11 @@ def _pre_setup(self):
def _urlconf_setup(self):
set_urlconf(None)
if hasattr(self, 'urls'):
+ warnings.warn(
+ "SimpleTestCase.urls is deprecated and will be removed in"
@timgraham
Django member
timgraham added a note Apr 6, 2014

Just missing spaces at the end of these strings - I've added them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Django member

merged in cd914e3.

@timgraham timgraham closed this Apr 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.