Opened 9 years ago

Last modified 3 years ago

#24622 new Bug

Response "context" and "templates" not available in the Test Client when using Jinja2 - Django 1.8

Reported by: rivantis Owned by: nobody
Component: Testing framework Version: 1.8
Severity: Normal Keywords: jinja2, test client
Cc: piquadrat@…, moritz.sichert@…, Roy Smith Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using Jinaj2 as the template of choice for a new Django 1.8 project, the Test Client does not populate the context and templates variables of the Response class.

If you run the same test using the Django Template Engine, everything works as expected.

I understand that the Django Test Client uses signals to populate the variables above, however as a someone new to Django, I was not able to pinpoint where the issues lies.

Attachments (1)

backwards_incompatible_test.py (575 bytes ) - added by Moritz Sichert 8 years ago.

Download all attachments as: .zip

Change History (29)

comment:1 by Aymeric Augustin, 9 years ago

Triage Stage: UnreviewedAccepted

Using signals serves us well :-(

I suppose we should:

  • make sure this feature works as expected when multiple Django template engines are available
  • add it to the jinja2 backend
  • document how other third party backends can use it

comment:2 by Preston Timmons, 9 years ago

I don't have bright ideas off the top of my head, but if we support this for other engines, there may be some caveats. The test runner patches django.template.base.Template._render to send a signal. This means templates from include tags, etc. also appear in the templates list. If it's somehow handled at the backend level, subsequent templates rendered internal to the engine won't appear in the list.

comment:3 by Aymeric Augustin, 9 years ago

The alternative would be to document these features of the test client as specific to the DTL and not support them in other engines.

comment:4 by Preston Timmons, 9 years ago

A documentation patch seems reasonable to me. Admittedly, though, I never use this feature or assertTemplateUsed, so I don't know how important having included and extended templates added is.

That gives us these options:

  1. Document that it doesn't work
  2. Add in support at the backend level for just the top-level template.
  3. Add instrumentation support at the backend level for included/extended templates.

Option 1 and 2 are straightforward. Option 3 is ugly but equivalent to what django.test.utils.setup_test_environment does to Django currently. It would need to be implemented via a sort of setup and teardown method on the backend that monkeypatches the internal render method.

I vote for option 1. My reason being that it's not worth partially implementing the feature nor monkeypatching Jinja2 to make it work fully.

comment:5 by Carl Meyer, 9 years ago

I talked to Armin in IRC today and I think there might be a variant of option (3) that wouldn't be too bad. Armin says that Environment._load_template, while marked as private API, is stable enough to safely override, and all template renders (not just top-level) go through it. (Disclaimer: I haven't looked at code in detail, just relaying here). So instead of monkeypatching Jinja2, we could maybe provide our own subclass of Environment that people can use if they want this feature. The main detail to work out would be whether this Environment subclass always fires signals or only does it during test; even if we enabled it via monkeypatch like we do for DTL, we could at least be monkeypatching our own code in the subclass rather than Jinja2 directly.

(Personally I think we should rework the DTL system to not rely on monkeypatching either, but this requires some design decisions. It may be that the signals framework is fast enough in the no-listeners case now that we can safely just always send these signals, instead of monkey-patching them in for testing only.)

comment:6 by Preston Timmons, 9 years ago

Thanks for looking into that, Carl.

First, I agree that it would be nice to remove the DTL monkeypatch. We might be able to do this by adding an enable_signals option to the Django backend. This way signals could be enabled or disabled via the TEMPLATES setting. I'll look into this further to see if I'm missing anything here. (#14365)

Second, while an Environment subclass for Jinja2 is definitely cleaner, it may be insufficient for the bigger use cases outside this ticket. From what I can tell, Environment._load_template doesn't have access to context variables. Django uses the same signal to populate response.context and response.templates. Context variables are also used in the Django debug toolbar template panel. I guess we could patch in the instrumentation at the instance level, here, though. That's not great but it's an improvement over changing Jinja2 internally.

Also, I suppose any signal here should send an instance of the backend template class rather than the internal template class. For assertTemplateUsed to work, we need to ensure the signalled template objects have a name attribute.

comment:7 by Carl Meyer, 9 years ago

Oh yes, Armin did mention that he intentionally doesn't provide any hooks into anything context-related after the initial render, so it seems that's not likely to change. A couple approaches he mentioned that we could look into:

`
22:11 < mitsuhiko> if you want to intercept the context somehow, you could do things
22:11 < mitsuhiko> but it's pretty ugly
22:11 < mitsuhiko> you can set a custom template_class on the environment
22:11 < mitsuhiko> and then in that template class you patch root_render_func and all funcs in blocks
22:11 < mitsuhiko> as they are passed the context
22:12 < mitsuhiko> maybe you also get away with overriding new_context
`

comment:8 by Benjamin Wohlwend, 9 years ago

Cc: piquadrat@… added

Seeing as it's probably too late to fix the actual functionality for 1.9, would a documentation update to the tune of "This attribute is only populated when using the DTL" be acceptable?. TemplateResponse responses have the context_data attribute, so that can be used instead in many cases.

comment:9 by Tim Graham, 9 years ago

Sure, you may send a pull request for that and just reference this ticket in the commit message "Refs #24622 -- ...".

comment:10 by Tim Graham <timograham@…>, 9 years ago

In 2b9eed4:

Refs #24622 -- Documented alternatives to some test response attributes when using alternative template engines.

comment:11 by Tim Graham <timograham@…>, 9 years ago

In c367abb1:

[1.9.x] Refs #24622 -- Documented alternatives to some test response attributes when using alternative template engines.

Backport of 2b9eed41fa26537d1af4f818c6e4296ce3305b01 from master

comment:12 by Tim Graham <timograham@…>, 9 years ago

In 840e97a:

[1.8.x] Refs #24622 -- Documented alternatives to some test response attributes when using alternative template engines.

Backport of 2b9eed41fa26537d1af4f818c6e4296ce3305b01 from master

comment:13 by Philipp Spitzer, 8 years ago

"This attribute is only populated when using the django.template.backends.django.DjangoTemplates backend" might be a bit misleading:
templates is populated with an empty list, context is set to None for non-Django templates.
As I see it, there is currently no way for 3rd party template engines to be compatible with the TestClient in this respect.

comment:14 by Tim Graham, 8 years ago

As I see it, "populated" means set to something non-empty/non-none, but I recognize the wording could be not ideal for non-native speakers. Feel free to submit a rewording if you like.

comment:15 by Philipp Spitzer, 8 years ago

Thanks! Maybe it's just me misunderstanding it... What about something like
"This attribute is set to an empty list when not using the django.template.backends.django.DjangoTemplates backend." for the templates attribute and "This attribute is set to None when not using the django.template.backends.django.DjangoTemplates backend." for the context attribute?

Initially I came across this bug report after spending an evening trying to find out why assertTemplateUsed does not work as expected (using a 3rd party template engine). Maybe a warning at https://docs.djangoproject.com/en/1.9/topics/testing/tools/#django.test.SimpleTestCase.assertTemplateUsed would be useful as well - especially for people who do not know how Django internally gets the information necessary for this test?

comment:16 by Tim Graham, 8 years ago

Sure, feel free to submit a pull request.

comment:17 by Moritz Sichert, 8 years ago

Cc: moritz.sichert@… added

Would patching django.template.backends.jinja2.Template.render() instead be a reasonable solution to the problem?
The advantage for that is that then future backends could be patched the same way.
The disadvantage is that when patching that method only the render() call to the first template will be caught, all renders triggered by the template itself (e.g. by using inheritance) not.

Let me know if that sounds reasonable, then I'll work out a PR.

comment:18 by Tim Graham, 8 years ago

I'm not sure if that's appropriate because you should probably aim to avoid executing the new logic when not in a testing situation.

in reply to:  18 comment:19 by Moritz Sichert, 8 years ago

Yeah, I mean monkey patching the render() method just like is done with the _render() method of django templates right now‌.

comment:20 by Tim Graham, 8 years ago

I guess I'd have to see the code as I don't understand what your suggesting.

comment:21 by Moritz Sichert, 8 years ago

I added a PR to show what I mean:
https://github.com/django/django/pull/6194

comment:22 by Simon Charette, 8 years ago

Has patch: set
Needs documentation: set
Patch needs improvement: set

The provided mock-up makes sense.

As noted by Aymeric this will require the addition of documentation to detail how third-party backend should use it.

by Moritz Sichert, 8 years ago

comment:23 by Moritz Sichert, 8 years ago

I attached a file that contains a test that will fail if we don't patch the DTL's internals. In the test derived.html contains the line {% extends 'base.html' %}.

However I'm not sure if that is something people want to do with template testing, on the other hand providing a deprecation warning is not that hard.

comment:24 by Moritz Sichert, 8 years ago

Has patch: unset
Needs documentation: unset
Patch needs improvement: unset

I just spoke to Markus about that and he says the way assertTemplateUsed() works should be consistent across template backends. That means that if it checks for base templates and included ones in the Django template engine (as it currently does) it should also work for the other backends like Jinja2.

To make this consistent across the two current backends and potential new ones Markus proposed adding a method called something like setup_test_environment() to each template backend class that (monkey-)patches the backend to send the template_rendered signal.

comment:25 by Gaige B Paulsen, 6 years ago

This limitation still exists with Jinja2 templates in 2.0.2, is there any interest in having somebody work on it? I ran into it this morning and it's something that I'm willing to put some work into fixing, assuming that there is an approved way forward, or a willingness to discuss one.

comment:26 by Roy Smith, 3 years ago

Is anything happening with this? I just got bit by it.

{% for hour in several %}
  {{ head.beat("wall") }}
{% endfor %}

Making this work would be nice, but if that's infeasible, at least mention the problem in the docs.

Version 0, edited 3 years ago by Roy Smith (next)

comment:27 by Roy Smith, 3 years ago

Cc: Roy Smith added

comment:28 by Roy Smith, 3 years ago

FWIW, here's the workaround I've been using. It monkey-patches django.shortcuts.render to send the required signal (part of this source file). The downside is it doesn't get you the full template chain, just the top-level one you call directly from your view. For my purposes, that's (mostly) good enough, but it's certainly not a plug-compatible replacement.

I say "mostly" because I'm now looking at a case where I need to see included templates. Sadly, I think this approach is too complicated to extend to support that, so I'm probably going to fall back to a low-tech solution: having each of my jinja templates emit a HTML comment confessing the template name and parse the generated HTML looking for those.

Django 3.1.12
Jinja 2.11.3
Python 3.7.3

from unittest.mock import patch

from django.test import TestCase
from django.test.signals import template_rendered
from django.shortcuts import render


class ViewTestCase(TestCase):
    """Base class for all SPI view tests.                                                                                                                    
                                                                                                                                                             
    Subclass this and have setUp() call super().setUp('spi.my_view')                                                                                         
    for a view defined in my_view.py.                                                                                                                        
                                                                                                                                                             
    """
    @staticmethod
    def render_patch(request, template, context):
        """Work-around for the django test client not working properly with                                                                                  
        jinga2 templates (https://code.djangoproject.com/ticket/24622).                                                                                      
                                                                                                                                                             
        """
        template_rendered.send(sender=None, template=template, context=context)
        return render(request, template, context)


    def setUp(self, view_module_name):
        render_patcher = patch(f'{view_module_name}.render', autospec=True)
        self.mock_render = render_patcher.start()
        self.mock_render.side_effect = self.render_patch
        self.addCleanup(render_patcher.stop)
Note: See TracTickets for help on using tickets.
Back to Top