Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add debug info for more lines #674

Closed
wants to merge 4 commits into from

Conversation

atodorov
Copy link
Contributor

Using the following template:

{% block body %}
  <ul>
  {% for user in users %}
    <li>Hello {{ user }}</li>
  {% endfor %}
  </ul>
{% endblock %}

jinja yields the following Python source code for it:

from __future__ import division, generator_stop
from jinja2.runtime import LoopContext, TemplateReference, Macro, Markup, TemplateRuntimeError, missing, concat, escape, markup_join, unicode_join, to_string, identity, TemplateNotFound
name = 'loop.html'

def root(context, missing=missing, environment=environment):
    resolve = context.resolve_or_missing
    undefined = environment.undefined
    if 0: yield None
    pass
    yield from context.blocks['body'][0](context)

def block_body(context, missing=missing, environment=environment):
    resolve = context.resolve_or_missing
    undefined = environment.undefined
    if 0: yield None
    l_0_users = resolve('users')
    pass
    yield '\n  <ul>\n  '
    for l_1_user in (undefined(name='users') if l_0_users is missing else l_0_users):
        pass
        yield '\n    <li>Hello %s</li>\n  ' % (
            l_1_user, 
        )
    l_1_user = missing
    yield '\n  </ul>\n'

blocks = {'body': block_body}
debug_info = '1=10&3=19&4=22'

Notice that debug_info is missing mappings for two more yield statements, the one on line 18, yield '\n <ul>\n ', and the one on line 25, yield '\n </ul>\n'. This in turn causes the Template.get_corresponding_lineno() method to return incorrect result.

@atodorov
Copy link
Contributor Author

With the proposed patch in the second commit debug_info becomes
debug_info = '1=10&1=18&3=19&4=21&4=22&5=25' which maps to all yield statements but doesn't map very well to the line numbers in the HTML template.

@atodorov
Copy link
Contributor Author

After the third commit the value is debug_info = '1=10&2=18&3=19&4=22&6=25' and the line numbers for some of the template nodes have been updated.

NOTE: please review the commits one by one to spot the differences. I'm not convinced this is a proper fix for my issue but being able to properly map between HTML lines in the template the generated Python source will enable proper coverage reporting for Jinja templates.

I do have a coverage.py Jinja plugin which appears to be working correctly if not for the fact that Jinja doesn't provide all the line numbers information to properly report on the results.

Note2: Template.get_corresponding_lineno() will not report proper results when lineno (from Python source) > code_line (from debug info mappings). It will just move to the next iteration and report that line as part of the previous line in the HTML. At the moment this makes coverage report zero-length loops as executed.

@mitsuhiko
Copy link
Contributor

Same as with the other PR #673 I do not quite comprehend why this is considered a bug. Under which circumstance would a yield statement for template data fail?

@mitsuhiko
Copy link
Contributor

mitsuhiko commented Jan 29, 2017

To make this more concrete: get_corresponding_lineno is only used for tracebacks. No error can be generated in the line you're referring to.

@atodorov
Copy link
Contributor Author

@mitsuhiko correct, no error can be generated here, but I need somehow to be able to map this line of Python code to its corresponding line in the template text. get_corresponding_lineno() sounds like the most obvious way to do this. Do you propose otherwise ?

I can jump on IRC or email thread if you guys prefer to discuss at length, just let me know.

@ThiefMaster
Copy link
Member

Knowing your usecase for having more detailed debug info would indeed be useful

@mitsuhiko
Copy link
Contributor

I should probably point out that get_corresponding_lineno is not a public API. I am not entirely sure what you are attempting to do but there has never been a commitment from out side to support get_corresponding_lineno.

@atodorov
Copy link
Contributor Author

@ThiefMaster I am working on coverage.py plugin for Jinja templates. It can be found at
https://github.com/MrSenko/coverage-jinja-plugin

The detailed debug info helps me get proper measurements as to which lines were actually covered.

Then "fixing" the node line numbers to match the lines in the HTML template will make it very easy to produce coverage reports (e.g. colored HTML or annotated text).

If it helps for the loop.html template shown above I get the following node structure (before any of the commits in this PR). Numers are the lineno attribute of the nodes. You see how they don't match the HTML text at all.

1:nodes.Template([
    1:nodes.Block('body', [
        1:nodes.Output([1:nodes.TemplateData('\n  <ul>\n ')]),
        3:nodes.For(3:nodes.Name('user', 'store'),
                    3:nodes.Name('users', 'load'),
                    [3:nodes.Output([
                        3:nodes.TemplateData('\n <li>Hello '),
                        4:nodes.Name('user', 'load'),
                        4:nodes.TemplateData('</li>\n  ')
                     ])
                    ], [], None, False
                   ),
        5:nodes.Output([5:nodes.TemplateData('\n </ul>\n')])
    ], False)
])

@jdahlin
Copy link

jdahlin commented Feb 9, 2017

I would also like to point out that, the jinja compiler will attempt to merge multiple TemplateData into one statement in the generated code, eg:

<div id="a">
   <div id=b"></div>
</div>

Will end up being compiled into something like this:

yield '<div id="a">\n <div id=b"></div>\n</div>'

That line requires multiple entries in debug info, executing it should tell the coverage plugin that 3 different template lines are in fact being covered.

This is likely to be more involved as template.get_corresponding_lineno() would actually return more than one line in these cases.

There are a few statements like this in jinja, with/endwith for instance, which do not have an obvious line in the compiled output. I wonder if the compiler should start emitting some kind of noop statement just to get better debug info generated.

@atodorov
Copy link
Contributor Author

@jdahlin can you tell me when does the jinja compiler generate debug info at the moment. By which property does the compiler know to generate debug info ? I may be able to use that in my tool to work around current limitations ?

@nzjrs
Copy link

nzjrs commented Apr 12, 2018

FYI https://github.com/MrSenko/coverage-jinja-plugin has been valuable in my testing and it would only be better if the HTML output could be made correct through the fix this patch provides.

@s0undt3ch
Copy link

We're actually using Jinja to "templatize" some yaml files and I was after some way to provide coverage information on those files.
I'm not saying jinja2 should report all lines, specially if it hurts performance, but maybe optionally provide them?

Anyway, this coverage approach does sound interesting.

@davidism davidism changed the title Bug [fix]: missing debug info add debug info for more lines Oct 18, 2019
@davidism
Copy link
Member

davidism commented Nov 4, 2019

Thanks for looking at this, sorry I took so long to get to it. I've been experimenting with this PR and better line mapping for a few days now, and my conclusion is that this isn't the right direction. It still misses multi-line template data, end blocks, and extension tags. And it adds more special-case debug_info code in hard to understand ways.

I recognize that there's interest in a coverage plugin for templates, but it turns out to be non-trivial given the way the lexer and compiler optimizations work. Perhaps we could add start and end lines to nodes, or base coverage on an analysis of the AST rather than the debug_info, but either way someone will have to investigate that more and provide a cleaner solution before I'm willing to add it.

@davidism davidism closed this Nov 4, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants