contrib.auth tests
Cam MacRae
seems a sensible default in line with Russell's post [1] but rev 8497
introduces a test only template directory [2] which
a) causes the tests to pass in the absence of an actual login template
(the provided template is not a default template, it's a test
template); and
b) overwrites the TEMPLATE_DIRS so that any user provided templates
must be in an app template directory. This is the case where
contrib.admin is installed.
#8523 has a patch with removes the overwriting of the TEMPLATE_DIRS
with a test only template dir, restoring the previously default
desired behavior but it was marked wontfix.
I added a patch which prepended the test template dir to
settings.TEMPLATE_DIRS, but the original patch is probably correct if
we agree that the view tests should fail if required templates aren't
found.
If this isn't the case, contrib.auth should provide default templates
rather than test only templates and testing user supplied templates
ought be the responsibility of the user.
Thoughts?
1. http://groups.google.com/group/django-developers/msg/e7c9ec81b209e01e
2. http://code.djangoproject.com/browser/django/trunk/django/contrib/auth/tests/views.py?rev=8497#L97
Malcolm Tredinnick
On Tue, 2008-08-26 at 18:09 -0700, Cam MacRae wrote:
> contrib.auth view tests fail if required templates aren't found. This
> seems a sensible default in line with Russell's post [1]
The problem with that original position is that it overloads testing to
include both testing the auth app's implementation and testing the
user's configuration when they're using the auth app. So there are
definitely two legitimate sides to that opinion.
I much prefer self-contained unittests. I read Russell's mail as
preferring to trade that for installation/configuration testing. I don't
particularly agree with that, but I could live with it if we decide it's
the way we want to go.
> but rev 8497
> introduces a test only template directory [2] which
>
> a) causes the tests to pass in the absence of an actual login template
> (the provided template is not a default template, it's a test
> template);
It also means the tests pass as a self-contained bundle to test the auth
app, rather than requiring a user using the auth app to have to set up
all these templates even if they're not using a particular piece of the
auth app just to have the tests pass. That's not a trivial concern.
There is a middle ground here, which is that those auth-app templates
get moved to the Django's main test directory so that they're available
only to runtests. That doesn't feel particularly self-contained and
again, for me, violates the intuitive meaning of the word unittest, but
it's the way we've gone with the new comment tests and the admin tests,
for example. Might be a trend to follow.
Malcolm
Cam MacRae
wrote:
> I much prefer self-contained unittests. I read Russell's mail as
> preferring to trade that for installation/configuration testing. I don't
> particularly agree with that, but I could live with it if we decide it's
> the way we want to go.
where it's come up it seemed to have been decided. If there is room to
move, I think that contrib apps should be self contained for test
purposes.
> It also means the tests pass as a self-contained bundle to test the auth
> app, rather than requiring a user using the auth app to have to set up
> all these templates even if they're not using a particular piece of the
> auth app just to have the tests pass. That's not a trivial concern.
>
> There is a middle ground here, which is that those auth-app templates
> get moved to the Django's main test directory so that they're available
> only to runtests. That doesn't feel particularly self-contained and
> again, for me, violates the intuitive meaning of the word unittest, but
> it's the way we've gone with the new comment tests and the admin tests,
> for example. Might be a trend to follow.
that anyone not using the contrib.auth views still needs to provide
the templates in their project?
Rather than test specific templates, why not provide a set of default
app templates? By doing so, the app is self contained, anyone not
using the views can forget the tests, anyone who is using them has the
freedom to use their own templates and the comfort that those
templates will be used for the tests.
cam.
Michael Richardson
> Rather than test specific templates, why not provide a set of default
> app templates? By doing so, the app is self contained, anyone not
> using the views can forget the tests, anyone who is using them has the
> freedom to use their own templates and the comfort that those
> templates will be used for the tests.
useful data without specifying the "registration" stuff. I use
contrib.auth without requiring passwords which invalidates a ton of
urls hooked into contrib.auth.urls - this means that all the tests
fail.
I would definitely prefer having self contained templates for
contrib.auth tests - I realize that if templates are to be included by
the users then it's great to have them automatically tested, but
there's so much functionality that can be had without requiring these
urls and views.
In other words, my vote is to set up fake templates for the testing.
Cam MacRae
> I use
> contrib.auth without requiring passwords which invalidates a ton of
> urls hooked into contrib.auth.urls - this means that all the tests
> fail.
> I would definitely prefer having self contained templates for
We use test templates and keep them in the module: We get self
contained tests at the expense of the user having to write their own
tests for contrib.auth views iff they use them. I guess that applies
to contrib.admin too. This is closest to what we have right now (post
rev 8497) with the advantage that users without contrib.admin
installed will have their project tests pass without having to create
an empty app for auth templates.
We use test templates and keep them in Django's main test directory.
contrib.admin will still require it's own auth view tests as the
contrib.auth tests will no longer function as integration tests for
admin, and users without contrib.admin installed will still need to
supply the templates for their project tests to pass.
We use default app templates and keep them in the module: we get self
contained tests which also cover user supplied templates (albeit where
EN copy changes require translation), but we lose the behavior where
the tests fail without a user supplied template.
I'll gladly do the work if we can come to decision as to which path to
follow.
cheers,
Cam.
Russell Keith-Magee
<mal...@pointy-stick.com> wrote:
>
> On Tue, 2008-08-26 at 18:09 -0700, Cam MacRae wrote:
>> contrib.auth view tests fail if required templates aren't found. This
>> seems a sensible default in line with Russell's post [1]
>
> The problem with that original position is that it overloads testing to
> include both testing the auth app's implementation and testing the
> user's configuration when they're using the auth app. So there are
> definitely two legitimate sides to that opinion.
>
> I much prefer self-contained unittests. I read Russell's mail as
> preferring to trade that for installation/configuration testing. I don't
> particularly agree with that, but I could live with it if we decide it's
> the way we want to go.
I suppose this just serves to highlight that there are two types of
testing that need to be performed - application tests and integration
tests. While the unittest framework is certainly well suited to
application testing, in a (semi) controlled environment like a Django
project it can also be quite useful for integration testing.
In a complete system, both forms of testing are required, which means
a complete reusable application should have two tests:
1) a test that the views work as expected when a known correct
template is provided
2) a test that the view works as expected with the template that has
been provided by the project (which is also a test that the required
templates are available).
The self-contained approach to testing is essentially case (1). This
is fine, but you still need to write a test for (2).
A well written 'template external' test _should_ validate both cases
when it passes. There are some weakness to this argument - if you get
a failure, it doesn't help you narrow down if the problem is your
template or the application logic, and there is the outside
possibility that a bad template could somehow cause the test to pass
(although I can't think of an easy way that this would happen) - but
as a general argument, it should hold.
The failure case isn't that bad, though, because there are two people
that will run the test -
* The application developer
* The end user
The application developer should have a good set of templates in their
test harness (such as Django's templates in the system test folder),
and if a failure is observed, the app developer is in the best
position to sort out the source of the failure.
End users should be able to trust their app providers to be testing
and tagging releases only when tests pass; if an end user is seeing a
failure, it's a reasonable assumption that the failure is template
related. As a side note, if the end user trusts their provider, there
is almost no reason for the end user to run type (1) application tests
(case in point - how often do non-core devs run the Django test suite?
How often to Django core devs run the MySQL test suite?)
Of course, the ideal solution would be a good answer for the problem
of supporting both application and integration tests, but that's part
of a much bigger discussion.
>> but rev 8497
>> introduces a test only template directory [2] which
>>
>> a) causes the tests to pass in the absence of an actual login template
>> (the provided template is not a default template, it's a test
>> template);
>
> It also means the tests pass as a self-contained bundle to test the auth
> app, rather than requiring a user using the auth app to have to set up
> all these templates even if they're not using a particular piece of the
> auth app just to have the tests pass. That's not a trivial concern.
The missing piece here is something that I have raised in the
discussion around #7611 - there will be tests that should be skipped
as part of the suite if the feature they are testing isn't actually
deployed in a project. For example, if you're not deploying the
password change view you don't need to run tests that check that the
password change view work is working correctly. Devin Naquin (from
Disqus) did some initial work in this area around ticket #4788; I
haven't had a chance to look into his patches yet, but it's on my todo
list once we get 1.0 finished.
> There is a middle ground here, which is that those auth-app templates
> get moved to the Django's main test directory so that they're available
> only to runtests. That doesn't feel particularly self-contained and
> again, for me, violates the intuitive meaning of the word unittest, but
> it's the way we've gone with the new comment tests and the admin tests,
> for example. Might be a trend to follow.
To my mind, moving the templates to the django test directory is the
right solution in this case. The Django system tests need to validate
that the auth system works when the right templates are available, so
it needs to provide those templates.
Unfortunately, this means that the auth tests will fail for users that
aren't providing auth templates. I realize that this isn't an ideal
outcome, but my gut feeling is that the benefit to users knowing their
installs are busted is greater than the pain for those users that will
have known test failures due to not deploying some views and
templates.
Yours,
Russ Magee %-)
Devin
> > There is a middle ground here, which is that those auth-app templates
> > get moved to the Django's main test directory so that they're available
> > only to runtests. That doesn't feel particularly self-contained and
> > again, for me, violates the intuitive meaning of the word unittest, but
> > it's the way we've gone with the new comment tests and the admin tests,
> > for example. Might be a trend to follow.
>
> To my mind, moving the templates to the django test directory is the
> right solution in this case. The Django system tests need to validate
> that the auth system works when the right templates are available, so
> it needs to provide those templates.
>
> Unfortunately, this means that the auth tests will fail for users that
> aren't providing auth templates. I realize that this isn't an ideal
> outcome, but my gut feeling is that the benefit to users knowing their
> installs are busted is greater than the pain for those users that will
> have known test failures due to not deploying some views and
> templates.
We essentially want to do two tests. Like you said (1) test the auth
app given a valid template and (2) test that the view is used
correctly by the project using the app.
(1) is a Django test. To ensure that contrib.auth is working if given
a correct template. This should live in runtests and use a known good
template.
(2) is of the use of that view. This should live in contrib.auth and
be skipped if that view isn't being used. Of course this depends on
the patch I've written in #4788. (And in fact, I've taken that
approach with those tests in that patch.)
If this approach is taken. We also need to add a test to runtests that
will essentially do the same thing, but provide a known good template.
Cheers,
Devin