contrib.auth's ChangePasswordTest tests have invalid setUp/tearDown methods

[mtrichardson]

This was introduced in #8497 by my patch - we shouldn't be changing the template directory at all on that since current behavior is that the tests should fail if these templates aren't present. However, ChangePasswordTest changes the TEMPLATE_DIRS setting, which we were using to make sure that our tests would run properly - this isn't actually relevant at all.

[mtrichardson]

Remove improper setUp/tearDown methods on ChangePasswordTest

[mtrichardson]

Additionally django/contrib/auth/tests/templates should be removed as well.

[Jacob]

The ChangePasswordTest.test_password_change_succeeds test case depends on that template being there, so I'm going to leave it.

[cammacrae]

Thumping the template_dirs means that anyone who uses django.contrib.auth without django.contrib.admin has a broken build - unless they move their templates into an app directory.

[cammacrae]

This patch prepends the test template directory to settings.template_dirs and as a bonus ensures the login method honors settings.LOGIN_REDIRECT_URL

[cammacrae]

See also here: ​http://groups.google.com/group/django-developers/browse_thread/thread/ac888ea8567ba466 for Russell's argument that the tests should fail in the absence of the templates. I actually think that given the test is now using it's own template_dir, the case could be made that contrib.auth tests should not depend on the contrib.admin templates at all.

[cammacrae]

just noticed that my patch fixes also #8552 which has a patch of its own. will new patch for this issue only to follow.

[cammacrae]

This patch prepends the test template directory to settings.template_dirs ONLY

[Malcolm Tredinnick]

@cammacrae: your patches are not what this ticket was about. The issue in this ticket has been resolved as "wontfix". Russell's post on the mailing list overlooked the problem that we need to be able to test things internally and making it impossible to run such tests as a way of catching errors in user-application configuration is certainly not the intention. It's a trade-off.

I'm reclosing this ticket with the original resolution. If you want to make these other changes, please open a new ticket, addressing the specific issue. As soon as you find yourself writing "as a bonus", you know you're trying to do two things at once (one of which, it turns out, is already addressed in another ticket). Sorry to be a curmudgeon on this, but it is very useful for us to be able to have one issue per ticket and not have a ticket side-tracked by something in the same area but not directly related to the original problem report.

Finally, from a quick read of your patch, if you want to go down that path, you'll almost certainly want the user configured template directories to be read before the standard ones (so that they override the standard ones). It doesn't look like that is going to happen.

[cammacrae]

Malcolm, you're really splitting hairs to say the patch doesn't address the ticket - contrib.auth's ChangePasswordTest tests do actually have an invalid setUp/tearDown methods. The first patch from mtrichardson is probably correct, but was rejected because the login.html template is required for tests to pass - neatly overlooking the fact that other templates are likewise required, but not included in the test template dir presumably for the reasons outlined in Russell's post. They are included, by seemingly happy accident, in contrib.admin. My patch addresses the broken setup method by ensuring that the tests can still pass in the absence of contrib.admin.

On the issue of the order of template directory resolution I'd agree with you if the templates used in the test were in fact standard templates for contrib.auth, but they're not: they're specific test templates (broken as that is).

As for the bonus, you're right & I knew you were right the second I uploaded the patch.

[Jacob]

Milestone 1.0 deleted