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

It's not finding templates on Windows 10 #46

Closed
tolomea opened this issue May 19, 2017 · 9 comments
Closed

It's not finding templates on Windows 10 #46

tolomea opened this issue May 19, 2017 · 9 comments

Comments

@tolomea
Copy link

tolomea commented May 19, 2017

I have following package versions:

coverage==4.4.1
Django==1.11.1
django-coverage-plugin==1.5.0
pytest==3.0.7
pytest-cov==2.5.1
pytest-django==3.1.2

.coveragerc contains:

[run]
plugins =
    django_coverage_plugin

Running pytest with:

pytest --cov-report term-missing --cov=core

correctly calculates coverage for the Python files in core but doesn't find any of the templates.
Running the same command, code and venv on a Linux machine works as expected and finds the templates.

@PamelaM
Copy link
Collaborator

PamelaM commented May 20, 2017

Thanks for the bug report, @tolomea

I don't have access to a windows system for testing, but I'm currently at pycon and I'll try to see if I can scrounge up someone to help on sprint Monday.

@tolomea
Copy link
Author

tolomea commented May 20, 2017

Let me know if there is any additional information I can provide or experiments I could do.

@Roang-zero1
Copy link

I am running into the same issue on my project. What would be needed to solve this problem?

@bugreport1234
Copy link

I've been running into the same issue. I believe I've found the cause of this bug.

In plugin.py line 151 the plugin gets the path to the django template library template directory by importing django.template and then accessing the __file__ attribute of the module. In line 169 it then compares every filename it is given to this path.

For some reason the path returned by __file__ doesn't have the exact capitalization of the actual Windows path to the module - the python standard library directory name Lib is returned as lib. I believe this is a bug in the CPython implementation. However I was not able to find anything on the CPython bugtracker about this, and I don't feel well versed in CPython internals enough to report it as a bug there. I found this question on stackoverflow which indicates that this has been an issue for a while, but unfortunately doesn't seem to have been considered as a bug by the answerer. If anyone wants to report it as a bug, please do so.

The way the __file__ capitalization causes this bug is when the filenames are compared in line 169 - the provided filenames do have the proper capitalization of Lib, and the check does a case-sensitive .startswith check, which then always fails, leading to the plugin never executing the code it needs to work.

An easy workaround/fix for the bug is to do the comparison case-insentively by converting both the path returned by __file__ and the provided filename to lowercase, either by calling the .lower() method, or by passing them to os.path.normcase() (which more or less does the same). I can confirm that this fixes the bug in my installation. However this might cause other problems when the paths contain Unicode characters, I haven't tested that.

@bugreport1234
Copy link

(This part is less directly related to the issue. I apologize if this is deemed as too off-topic.)

While looking through the plugin source to find the cause of this bug, I got a bit puzzled by the way the plugin works. From what I understand from the code, it registers itself to trace every file in django.template and then tries to pull out the template paths, as well as info about node boundaries, from any function called render with appropriate function locals when it is called, and uses this info to build the coverage data.

This approach seems a little odd to me, especially as the plugin doesn't actually intend to trace the contents of django.template, but rather the templates themselves, which coverage.py doesn't handle. This appears to me like it could be rather inefficient, and would run into problems once the Django developers introduce another function somewhere in the code that is also named render and has similar function locals.

Since the information of which parts of the template get traced and which ones aren't can be inferred from which Nodes have render called, wouldn't it also be possible to accomplish template coverage by monkey-patching nodes' render method with a decorator-like function that records that the node was executed (including the boundary information), and then passes the call through to the original render method?
I admit that this method would be more of a hack than an actual solution, but it seems cleaner to me than the current approach.

Again, I apologize for digressing from the original bug, and I don't want to make it sound like I'm trying to tell the maintainers what to do, I just now stumbled upon this project and so don't know the whole picture.

@jambonrose
Copy link
Collaborator

Thanks @bugreport1234! That's very helpful.

It sounds like running CI tests on Windows testing just got added to my to-do list. Some of the code provided by Martijn Pieters in the Stack Overflow looks to be directly relevant, too. Thank you for linking directly to the issue!

I can't speak for the other co-maintainers, but I'm currently focused on trying to fix bugs and add documentation. Once we're stable and have tests for all of the current issues, I'd be happy to look at performance improvements or a refactor of the code, but can't commit to a timeline on that front. Thanks for bringing it up, though!

@nedbat
Copy link
Owner

nedbat commented May 23, 2021

I'm putting Windows CI in place (as part of moving from Travis to GitHub Actions), and the test suite is failing. In my case, it's because of these different environment variable definitions:

HOME = C:\Users\runneradmin
TEMP = C:\Users\RUNNER~1\AppData\Local\Temp
TMP = C:\Users\RUNNER~1\AppData\Local\Temp

I don't know why TEMP is a shortname while HOME is a longname. But this breaks a comparison happening in coverage.py.

@nedbat
Copy link
Owner

nedbat commented May 23, 2021

I think this is now fixed in d287548. Let me know if it does or doesn't work.

@nedbat nedbat closed this as completed May 23, 2021
@nedbat
Copy link
Owner

nedbat commented Jun 8, 2021

This is now released in v2.0.0: https://pypi.org/project/django-coverage-plugin/2.0.0/

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

No branches or pull requests

6 participants