Tests for contrib.auth fail if login signals use templates

[jMyles]

The tests for contrib.auth override settings.TEMPLATE_DIRS in order to use the test templates, which are obviously required in order to have cognizable tests of the login system.

However, if a project uses the user_logged_in signal to hook a function that uses django.template.loader.get_template(), the test will improperly raise TemplateDoesNotExist.

Instead of ovverriding settings.TEMPLATE_DIRS, it makes more sense to simply create a new tuple, the first member of which is the location of the test templates.

[Julien Phalip]

This makes sense. Could you provide a test case reproducing this TemplateDoesNotExist exception? Thanks!

[anonymous]

I thought about providing a test, but I can't immediately think of a way to do it that is settings-independent. Shall I 'fake it' by appending to settings.TEMPLATE_DIRS in the actual test? I don't know of any other tests that employ this technique, so I'm not especially comfortable with it. If you can point me to one that I can use as a reference for good practice, I'd be happy to.

[Julien Phalip]

This paradigm (overriding TEMPLATE_DIRS in setUp()) is a common way for *unit* tests to be self-contained and runnable in any environment. What's tricky here is that auth sends the user_logged_in signal but has no control over how that signal is used outside of the auth app during testing. The core issue is that there is no easy way of separating *unit* tests from *integration* tests in Django. Ideally when running tests from your project, you should only be interested in running integration tests and avoid running the various apps' unit tests -- So one approach is to simply exclude the auth tests from your project's test runner.

Another approach would be to make the auth's tests even more self-contained by initially backing up and deactivating the currently registered signal receivers, then registering its own temporary receiver for the pure purpose of unit testing, and finally restoring the backed up receivers. That way the auth tests wouldn't fail, but you probably would still have to write integration tests in your own project to verify that your custom signal receiver works as intended.

[Preston Holmes]

I think the ultimate fix will be related to a full reorganization of the way tests are run in Django (this needs a ticket of its own). There has been recent discussion in IRC about replacing the test runner with something closer to a standard unittest runner, nose etc and making tests even more atomic and less coupled to a full suite run.

I think julien's explanation above is the best we have in the current situation. To patch this for auth doesn't solve the bigger problem of the mingling integration and unit tests.

as such I think this merits a wontfix.

[anonymous]

Although the larger issue of conflation of integration and unit testing is an important topic of discussion, it seems to me that this patch makes good sense no matter what path is chosen. In fact, it seems to me that overriding settings.TEMPLATE_DIRS is simply an oversight; I don't see that it reflects any underlying intention to separate unit tests from integration tests.

In any case, this patch will not inhibit a later, more perfect solution, and will likely be part of it, no?

The consequence of failing to accept this patch or one like it is that anybody using a template with the login signal is going to have errors in the testrunner. Is that OK?

[Aymeric Augustin]

The bug described actually exists, and we intend to fix it, even if that involves complicated work on test isolation. That's why I'd prefer to keep this ticket open (or close it a a duplicate of a more general ticket) at this time. I'll mark it as DDN because I can't say exactly how we'll fix it.

Note that the contributing guide states:

Please don’t close tickets as “wontfix.” The core developers will make the final determination of the fate of a ticket.

We don't enforce this rule strictly, but in debatable cases, please say why you think the proper resolution is "wontfix" and put the ticket in DDN.

[Julien Phalip]

For the record, I think this is just a symptom of a deeper problem, i.e. that Django doesn't provide a clean environment for unit tests to be run in. Other examples of such symptomatic issues are: #17194, #13394, #16366, #10976, #8404, #16413, and many more. Whatever we do to fix those symptoms we will always keep finding more and more of them — this just has no end as it's impossible to predict all the combinations of settings people might use in the wild with their projects. Patching those tests with little fixes does make them more isolated, but it also does make the tests harder to read as it increases the amount of boilerplate code inside the tests, without doing anything really useful like increasing the code coverage or fixing actual bugs in the tested code.

As stated earlier in this thread, I see no reason for running the Django contrib apps from one's project with 'manage.py test' at all. Those tests are already run by the CI server, and running them from your project will add no value to you. They can only cause annoyances when they fail because of isolation problems. This is why I recommend to simply ignore those tests and not run them from your project.

I contend that our energy would be better spent focusing on finding a more general solution to the root cause. I don't think much work has been done in that area yet. I know at least of #9156 addresses that.

[Aymeric Augustin]

The decision to override templates was a consequence of #10818.

[Aymeric Augustin]

My comment on #11077 applies here too.

[Aymeric Augustin]

Marking as accepted because it's a bug that needs to be addressed one way or another.

[Carl Meyer]

I believe this ticket can simply be closed when we merge #17365, since that will change the default behavior of manage.py test so it doesn't run contrib tests, making it feasible for us to simply say "contrib app tests aren't intended to pass with arbitrary settings."

[Carl Meyer <carl@…>]

In 9012833af857e081b515ce760685b157638efcef:

Fixed #17365, #17366, #18727 -- Switched to discovery test runner.

Thanks to Preston Timmons for the bulk of the work on the patch, especially
updating Django's own test suite to comply with the requirements of the new
runner. Thanks also to Jannis Leidel and Mahdi Yusuf for earlier work on the
patch and the discovery runner.

Refs #11077, #17032, and #18670.

[Carl Meyer]

Closing wontfix. Since the new test runner does not run contrib app tests in user projects by default, we will no longer be attempting to make contrib app tests pass with arbitrary combinations of settings, signals, templates, etc.