#7611 closed (wontfix)
auth app PasswordResetTest requires admin app to be installed
Reported by: | Andrew Badr | Owned by: | Jason Yan |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The tests added in http://code.djangoproject.com/changeset/7808 fail when the admin app is not installed. Tests for one app should not depend on the presence of another. The error is: TemplateDoesNotExist: registration/password_reset_form.html
Attachments (5)
Change History (16)
comment:1 by , 16 years ago
Component: | Uncategorized → Unit test system |
---|---|
milestone: | → 1.0 |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 16 years ago
comment:3 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
by , 16 years ago
Attachment: | test_template_loader_r7958.diff added |
---|
comment:4 by , 16 years ago
Has patch: | set |
---|
Attached patch adds a test template loader (resides in the test package right now, I wasn't sure if it would be useful enough to live in template/loaders). This currently uses the TEMPLATE_DIRS setting to pass the dictionary to the loader.
by , 16 years ago
Attachment: | test_template_loader_r7967.diff added |
---|
by , 16 years ago
Attachment: | test_template_loader_r7967-2.diff added |
---|
Previous patch missed a template change.
by , 16 years ago
Slightly different approach to that originally described.
comment:7 by , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
I've provided a slightly patch updated to post-newforms-admin merge, with a slightly modified approach - the implementation of the template loader convinced me that a dictionary syntax probably isn't the right thing to do here, and munging TEMPLATE_DIRS to be a dictionary could have all sorts of consequences.
However, after working on this patch and thinking about it a bit more, I'm not convinced that this is the right thing to do. I'm now of the opinion that the reported problem is in fact a correct test failure.
In the reported circumstances (auth deployed, but not admin), the auth test is failing because certain templates are not found.
When auth is tested as part of the Django system tests, admin is also deployed, so the templates are found.
The tests would pass for the reporter if the user provided a 'registration/password_reset_form.html' template in their own project.
If the reporter _had_ deployed contrib.admin, but _didn't_ use the app_directories template loader, the tests would have failed, because the ability to load the admin templates is dependent on the use of the app_directories template loader.
Essentially, as currently defined, the auth tests are validating that the auth application will work _in your current project_. If you're not providing a way to load a template called 'registration/password_reset_form.html', then the auth views will fail in your project, and the failure is correctly reported. It doesn't matter if you are providing that template yourself, or if it is loaded from admin, but it must be provided.
This test also serves as a check that when a template that _is_ provided, it is correct - in this case, that it renders the form, and displays the right error message. It's not enough that you have a template - you have to have a meaningful template, too. You don't have to use the admin password_reset_form.html template, but whatever you use, it has to render the form, and display the errors on the email field.
This does raise the slight problem that the 'missing' templates are required, even if you aren't using the auth views in your project. However, I feel that this is probably an edge case that we can live with.
If anyone disagrees with this analysis, feel free to raise it on django-dev, but for the moment, I'm closing wontfix. Apologies to Jason who worked on this ticket after my initial advice. Hopefully you gained some experience with the test and template systems, even if that experience isn't going to be put to use for the moment.
by , 16 years ago
Attachment: | 7611_r7977.diff added |
---|
comment:9 by , 16 years ago
I'm not sure wontfix is the right solution. Many people use the models defined in the auth app without using the views (and hence not providing the templates). The tests will fail in this case. Either the tests have to be deleted or dummy templates provided or the admin app added, none of which is a particularly nice solution.
comment:10 by , 15 years ago
Seems strange that we allow a testcase to locally override urls on the assumption that a project may have its own set of urls, but not allow a testcase to locally override templates even though a project is likely to have its own set of templates. Doing one and not the other actually raises its own bit of problems (which I have come across). An example would be a login template that has a url tag referencing another app. This url tag will not resolve as the test case has its own set of urls, thus a failure occurs. Either way I am looking forward to this being resolved, I have two test failures lingering due to this bug ;)
The problem isn't that admin is required - it's that the test requires templates that may not be available. Admin just happens to provide those templates.
I suspect what is needed here is a 'templates' attribute on TestCase, similar to the newly added 'urls' attribute:
The TestCase would then install a simple template loader to extract the templates from the text.