#25848 closed Cleanup/optimization (fixed)
Django 1.9 breaks django_coverage_plugin, template coverage doesn't work
Reported by: | Ned Batchelder | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | 1.9 |
Severity: | Normal | Keywords: | |
Cc: | sdeibel@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
django_coverage_plugin (https://github.com/nedbat/django_coverage_plugin) does some tricky things to figure out which lines in your templates are being used, and which are not. It works great in Django 1.4 through 1.8. The template refactoring in 1.9 removed information the plugin needs to work.
Preston Timmons has been very helpful in diagnosing the problem. He says the commit that broke it is: https://github.com/django/django/commit/55f12f8709f0604df7e1817a4c114ead1fb9a311
Preston's fuller assessment is:
Yes, I'm supportive of that going into 1.9.1. Do you want to make a ticket in
Trac? Please point to the commit where I broke things for you and mention how
token.source used to give you access to the current template, whereas
context.template.origin only gives you access to the parent. This was a change
in private API, but you have an important use case that's worth making sure you
can access the information you need.
.
A proper fix may include changing the semantics of context.bind_template.
Alternatively, we could expose the current template on context.render_context.
That attribute exists to store state during render invocations.
.
A less pleasant possibility may be to accesscompiled_parent.origin.name
in
the ExtendsNode render method. Or to grab the latest origin from the extends
history withcontext.render_context[ExtendsNode.context_key][-1]
. The
IncludeNode would require a similar hack, but hopefully we can give you
something better to work with than that.
I'm very willing to work out a good solution, and do some of the work on the Django side if need be. It would be a shame if 1.9 had to sit out this feature of coverage.py.
Change History (11)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 9 years ago
As an aside, I wonder if you tried to test with the prereleases of Django 1.9 and if not, if there's any way we could have helped to better facilitate that? If it's just a lack of time on your end, I certainly understand that.
comment:3 by , 9 years ago
I'm not confident if these are the right solutions yet, but I have a couple ideas:
First, we could update Template.render
to look like:
class Template(object): def _render(self, context): with context.bind_render_template(self): self.nodelist.render(context)
This would set a new attribute on context, render_template
. This would reference the template of the current render call. This change would need to occur in _render
rather than render
to work with both ExtendsNode and IncludeNode.
Alternatively, we could update ExtendsNode and BlockContext to store the origin along with each node. When Ned encounters a BlockNode, he could find the template origin by looking it up in the block context.
comment:4 by , 9 years ago
Tim, thanks. In this case, it was mostly just poor timing between when my attention came back around to Django template coverage, and when 1.9 came out. But partly, it was not realizing that template changes were underway, and being complacent that since my code was working on 1.4 through 1.8, it would probably be fine. :)
comment:5 by , 9 years ago
Preston, I'm assuming that you will make a proposal about the best way to proceed. I don't know enough to have an opinion between the two ideas you've laid out here. Or, if you need me to do more, let me know.
comment:6 by , 9 years ago
We missed the possibility of getting into 1.9.1. Preston, what should I expect from you? I'm not trying to nag, I know what it's like to be on the other side of that question! :)
Is 1.9.2 do-able?
comment:7 by , 9 years ago
FWIW I've run into this problem also in Wing IDE's support for debugging Django templates. I was able to partially solve the problem by looking up the stack for the nearest frame in Template and using self.origin.name. However, when a block is overridden via 'extends' I've not been able to figure out how to get the correct file name.
For example context.render_context[ExtendsNode.context_key][-1].name is still the wrong template.
Is there any reason that the template name can't just be put into self.token? Currently you have token.position but not token.filename and it seems position is pretty useless w/o the filename, as far as I can tell.
comment:8 by , 9 years ago
Cc: | added |
---|
I'll be happy to backport the solution to 1.9.