DDN: Contrib apps testing their own views (#7521)

5 views
Skip to first unread message

Marc Fargas

unread,
Jun 24, 2008, 1:17:41 PM6/24/08
to django-d...@googlegroups.com
Hi all,
You may have noticed that [7716] raised a few bugs around (#7514, #7517
and #7521) the nice one is #7521, which is now marked DDN.

The issue raised by this ticket is that "mange.py test" would fail to
run tests when you had contrib.auth in your INSTALLED_APPS. That is
because [7716] introduced and urls.py for testing contrib.auth views.
And there's no guarantee that the user's project will have those urls
included, which would lead to failed tests.

That means, that right now, contrib apps cannot test their views because
they could lead to failed tests on projects. (i.e. [7716] was reverted
in [7726] to avoid upsetting users).

But oviously, contrib apps should (to not say must) test their views to
be sure that code released works! Hence, there should be a way for
contrib apps to have an urls.py and test it.

I can think of two options right now:
* manage.py test should not run django.contrib.* tests, those are
supposed to have been run in runtests.py and working fine (that's the
point of being a contrib app: django developers take care of it!).
* mange.py test should make sure that django.contrib.*.urls are
included and disable tests that would fail, or include such urls.

I'd really be +1 on the first options, there's no point on testing
contrib apps in the user's project (for the same reason we do not run
the whole testsuite from the users project).

Side note, any NFA developer: when you merge from trunk please clean
django/contrib/auth/tests/forms.py#L23 as #7514 fixed this on trunk.
--
http://www.marcfargas.com -- will be finished some day.

signature.asc

Russell Keith-Magee

unread,
Jun 25, 2008, 6:43:11 AM6/25/08
to django-d...@googlegroups.com
On Wed, Jun 25, 2008 at 1:17 AM, Marc Fargas <tele...@telenieko.com> wrote:
>
> But oviously, contrib apps should (to not say must) test their views to
> be sure that code released works! Hence, there should be a way for
> contrib apps to have an urls.py and test it.

Agreed. This is something we definitely need to address.

> I can think of two options right now:
> * manage.py test should not run django.contrib.* tests, those are
> supposed to have been run in runtests.py and working fine (that's the
> point of being a contrib app: django developers take care of it!).
> * mange.py test should make sure that django.contrib.*.urls are
> included and disable tests that would fail, or include such urls.

I'm not sure I like either of these options.

There is some value in having end users run tests for django.contrib -
every person that runs the tests is one more person likely to spot
(and report, and maybe fix) a problem. I'm not a big fan of making
special cases, either.

My preferred solution here would be to provide a way for test cases to
substitute a top level URL pattern object for the duration of the
test. For example:

from django.tests import TestCase
class MyTest(TestCase):
fixtures = ['test.json']
urls = 'local.test_urls'
...

The setup methods could swap in a temporary root URLconf, and swap it
out again on teardown. The problem only exists for tests that need to
poke views, so there isn't a huge need for a doctest analog, but the
calls made by TestCase.setup/teardown could be abstracted into
test.utils.

Opinions?

Yours,
Russ Magee %-)

Gary Wilson Jr.

unread,
Jun 25, 2008, 10:40:18 AM6/25/08
to django-d...@googlegroups.com
Russell Keith-Magee wrote:
> On Wed, Jun 25, 2008 at 1:17 AM, Marc Fargas <tele...@telenieko.com> wrote:
>> I can think of two options right now:
>> * manage.py test should not run django.contrib.* tests, those are
>> supposed to have been run in runtests.py and working fine (that's the
>> point of being a contrib app: django developers take care of it!).
>> * mange.py test should make sure that django.contrib.*.urls are
>> included and disable tests that would fail, or include such urls.
>
> I'm not sure I like either of these options.
>
> There is some value in having end users run tests for django.contrib -
> every person that runs the tests is one more person likely to spot
> (and report, and maybe fix) a problem. I'm not a big fan of making
> special cases, either.

Agreed. I think manage.py test should run tests for all installed apps,
no matter if they are contrib apps or not.

> My preferred solution here would be to provide a way for test cases to
> substitute a top level URL pattern object for the duration of the
> test. For example:
>
> from django.tests import TestCase
> class MyTest(TestCase):
> fixtures = ['test.json']
> urls = 'local.test_urls'
> ...
>
> The setup methods could swap in a temporary root URLconf, and swap it
> out again on teardown. The problem only exists for tests that need to
> poke views, so there isn't a huge need for a doctest analog, but the
> calls made by TestCase.setup/teardown could be abstracted into
> test.utils.

Swapping the root URLconf works, and is currently what contrib.formtools
is doing. Is your thinking that if a ``urls`` attribute is defined, the
URLconf swapping in setup/teardown would be done for you?

Gary

Russell Keith-Magee

unread,
Jun 25, 2008, 6:58:51 PM6/25/08
to django-d...@googlegroups.com
On Wed, Jun 25, 2008 at 10:40 PM, Gary Wilson Jr. <gary....@gmail.com> wrote:

>
> Russell Keith-Magee wrote:
>> My preferred solution here would be to provide a way for test cases to
>> substitute a top level URL pattern object for the duration of the
>> test. For example:
>
> Swapping the root URLconf works, and is currently what contrib.formtools
> is doing. Is your thinking that if a ``urls`` attribute is defined, the
> URLconf swapping in setup/teardown would be done for you?

Thats pretty much what I was thinking. I didn't know formtools was
already doing it; that proves the concept quite nicely.

Russ %-)

Marc Fargas

unread,
Jun 27, 2008, 4:29:20 AM6/27/08
to django-d...@googlegroups.com
El jue, 26-06-2008 a las 06:58 +0800, Russell Keith-Magee escribió:
> >> My preferred solution here would be to provide a way for test cases to
> >> substitute a top level URL pattern object for the duration of the
> >> test. For example:
> >

A patch in these lines is now attached to #7521, the patch right now:
* Adds the "urls" parameter, and a new method to TestCase called
"_post_teardown", it's called at the end of __call__.
* Adds a "clear_url_cache()" method to django.core.urlresolvers (To
clear the resolver cache when swapping urlconfs)
* Adds two TestCases to check the "urls" parameter, they must be run
in order...
* modifies fomtools.tests to use the urls paremeter instead of it's
own hack (which should be broken somehow for the cache thing).
* docs!!!

The only thing not added to the patch is reverting [7726], it's on my
git branch thought,
http://www.marcfargas.com/gitweb/?p=django.git;a=shortlog;h=trac/7521-test-urlconfs


Hope this patch is ok!
Marc

signature.asc

Russell Keith-Magee

unread,
Jun 27, 2008, 4:40:45 AM6/27/08
to django-d...@googlegroups.com
On Fri, Jun 27, 2008 at 4:29 PM, Marc Fargas <tele...@telenieko.com> wrote:
> El jue, 26-06-2008 a las 06:58 +0800, Russell Keith-Magee escribió:
>> >> My preferred solution here would be to provide a way for test cases to
>> >> substitute a top level URL pattern object for the duration of the
>> >> test. For example:
>
> A patch in these lines is now attached to #7521, the patch right now:

I saw this last night - I had a quick look at the patch, and it looked
ok. Getting this into trunk is on the top of my todo list, which means
it should happen in the next day or two.

Yours,
Russ Magee %-)

Reply all
Reply to author
Forward
0 new messages