Skip to content
New issue

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

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

Already on GitHub? # to your account

Spurious missing lines reported if template has CRLF line endings #75

Open
sparrowt opened this issue May 10, 2021 · 2 comments
Open

Spurious missing lines reported if template has CRLF line endings #75

sparrowt opened this issue May 10, 2021 · 2 comments

Comments

@sparrowt
Copy link

sparrowt commented May 10, 2021

Summary

If the Django template HTML file has CRLF line endings (\r\n) then, depending on line and file lengths, some random lines may be reported as 'missing' even though they are covered. In my experience it happens more often on short lines later in the file, e.g. some closing HTML tags near the end.

Changing the line endings to LF-only (\n) in an affected file avoids the problem (reports 100%) - the bug is definitely because of the CRLF in the HTML template file, see 'Cause' section below. But first a minimal repro...

Repro

  • Running on Windows 10, Python 3.8.5 64-bit (though I expect this reproduces on linux & any recent python)
  • Download and unzip issue75_minimal_repro.zip, create a venv & install `requirements.txt
  • Run these commands inside the venv:
coverage run --source=./mysite/ mysite/manage.py test mysite
coverage report --include=*.html --show-missing
  • Output is as follows (rather than 100% as expected):
Name                                Stmts   Miss  Cover   Missing
-----------------------------------------------------------------
mysite\myapp\templates\index.html      13      1    92%   13
-----------------------------------------------------------------
TOTAL                                  13      1    92%

where line 13 of index.html simply contains </html>

  • Change the line-endings of minimal_repro\mysite\myapp\templates\index.html to be LF-only
  • Run the above 2 commands again - this time the result is 100% as expected

Cause

I believe the problem is caused by this plugin comparing character offsets (from start-of-file) into the template source string, loaded from 2 different bits of code which do it slightly differnetly:

  1. django_coverage_plugin.plugin.read_template_source specifies binary mode: open(filename, "rb") so this plugin is loading the HTML source including 2 characters \r\n at the end of each line if the file is CRLF
  2. django.template.loaders.filesystem.Loader.get_contents doesn't specify binary mode, so python 3 universal newlines behaviour results in the loaded HTML source having only 1 character \n at the end of each line, even though the file on disk has CRLF line endings

The comparison mismatch occurs because:

  • get_line_map calculates things based on (1) above (read_template_source) so includes \r
  • get_line_number looks up in that line map but based on s_start & s_end which come from position which comes from Django which has read the template source without the \r as per (2) above

Therefore, depending on the length of each line, as it progresses down the file, offsets start to land on the wrong line as the off-by-one errors stack up.

Solution

I guess in this plugin we need to do one of these:

  • (a) modify read_template_source to match Django's behaviour of getting universal newlines auto-fixed-up
  • (b) or is it possible to import & use Django's template loader directly so as to avoid future possible mismatch?

If (a) then it would be nice - if possible - to add a sanity check that the length of the string returned by read_template_source is the same as the length of the template that Django has loaded, if we can access that.

@johnlarusic
Copy link

Just adding that this exact same issue is happening to me. Going to see if I can force VSCode to just use the Unix-style line endings everywhere, but this fix would still be very much appreciated.

@sronveaux
Copy link

Hi,

Same problem here and very happy to see there is a workaround. Is there is a plan to make a fix for this one in an upcoming version?

Thanks for the wonderful job made on this plugin.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants