[Django] #24622: Response "context" and "templates" not available in the Test Client when using Jinja2 - Django 1.8
Django
using Jinja2 - Django 1.8
-----------------------------------+---------------------------------
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Severity: Normal | Keywords: jinja2, test client
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:1>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by prestontimmons):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:2>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
The alternative would be to document these features of the test client as
specific to the DTL and not support them in other engines.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:3>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by prestontimmons):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:4>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
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.)
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:5>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by prestontimmons):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:6>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
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
```
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:7>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by piquadrat):
* cc: piquadrat@… (added)
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:8>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Sure, you may send a pull request for that and just reference this ticket
in the commit message "Refs #24622 -- ...".
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:9>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"2b9eed41fa26537d1af4f818c6e4296ce3305b01" 2b9eed4]:
{{{
#!CommitTicketReference repository=""
revision="2b9eed41fa26537d1af4f818c6e4296ce3305b01"
Refs #24622 -- Documented alternatives to some test response attributes
when using alternative template engines.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:10>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"c367abb11be9f6758e87df3c178a77af4c5ba1bc" c367abb1]:
{{{
#!CommitTicketReference repository=""
revision="c367abb11be9f6758e87df3c178a77af4c5ba1bc"
[1.9.x] Refs #24622 -- Documented alternatives to some test response
attributes when using alternative template engines.
Backport of 2b9eed41fa26537d1af4f818c6e4296ce3305b01 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:11>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"840e97ab018ef0901fe4731d7502a7e2baf6546b" 840e97a]:
{{{
#!CommitTicketReference repository=""
revision="840e97ab018ef0901fe4731d7502a7e2baf6546b"
[1.8.x] Refs #24622 -- Documented alternatives to some test response
attributes when using alternative template engines.
Backport of 2b9eed41fa26537d1af4f818c6e4296ce3305b01 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:12>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by phispi):
"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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:13>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:14>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by phispi):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:15>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Sure, feel free to submit a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:16>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MoritzS):
* cc: moritz.sichert@… (added)
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:17>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:18>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MoritzS):
Yeah, I mean monkey patching the `render()` method just like is done with
the `_render()` method of django templates right now.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:19>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I guess I'd have to see the code as I don't understand what your
suggesting.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:20>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MoritzS):
I added a PR to show what I mean:
https://github.com/django/django/pull/6194
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:21>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_docs: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:22>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Changes (by MoritzS):
* Attachment "backwards_incompatible_test.py" added.
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Comment (by MoritzS):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:23>
Django
using Jinja2 - Django 1.8
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MoritzS):
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
* needs_docs: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:24>
Django
using Jinja2 - Django 1.8
-------------------------------------+------------------------------------
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Severity: Normal | Resolution:
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Gaige B Paulsen):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:25>
Django
using Jinja2 - Django 1.8
-------------------------------------+------------------------------------
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Severity: Normal | Resolution:
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Roy Smith):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:26>
Django
using Jinja2 - Django 1.8
-------------------------------------+------------------------------------
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Severity: Normal | Resolution:
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
* cc: Roy Smith (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:27>
Django
using Jinja2 - Django 1.8
-------------------------------------+------------------------------------
Reporter: rivantis | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Severity: Normal | Resolution:
Keywords: jinja2, test client | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Roy Smith):
FWIW, here's the workaround I've been using. It monkey-patches
django.shortcuts.render to send the required signal (part of
[https://github.com/roysmith/spi-
tools/blob/5ed0cd8f2285698f31f3355b9afa55f64f19bbab/spi/test_views.py 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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24622#comment:28>