-
Notifications
You must be signed in to change notification settings - Fork 36
Description
At first I would like to thank you for this wonderful plugin - we love it! We noticed the CRLF vs. LF problem a while ago but that just occured accidentally when we where testing through Gitlab Web IDE - so that's not a real blocker.
We recently looked closer into if conditions and think there is room for improvement, as sometimes 100% coverage is reported, but it just isn't true.
Let's create a very simple example: Imagine you could pass in a get param ?red=1, which would e.g. highlight the title:
<h1
{% if request.GET.red %}
class="red"
{% endif %}
>Headline</h1>
When you don't have a test covering passing in red=1 at least line 3 will be correctly reported as not covered. So far so good.
Sadly this unpopular but possible case
<h1
{% if request.GET.red %}
class="red" {% endif %}
>Headline</h1>
or this very popular case
<h1 {% if request.GET.red %}class="red"{% endif %}>Headline</h1>
will report 100% coverage - which is not correct at all.
This caught my attention, so I thought I should review it in Python in the view logic as well. Let's pretend the code above would rely on a variable:
<h1 {% if red %}class="red"{% endif %}>Headline</h1>
In our example we'd like to achieve the same behaviour so our view would look like this
def index(request):
red = False
if request.GET.get("red", False):
red = True
return render(request, "base/index.html", {"red": red})
Again this works as expected through pytest-cov, so in case you don't hand over ?red=1 in one of your the tests it will put line 4 on your list.
But sadly even in this extremely popular pattern it is not working as expected and stating 100% coverage, even when the condition is not covered.
def index(request):
red = False if request.GET.get("red", False) else True
return render(request, "base/index.html", {"red": red})
I checked it again with a colleague and still: maybe we've made a mistake somewhere, but these examples are very straight forward... so we doubt it. Could somebody spent a couple of minutes and confirm this?
We used:
- Python 3.12.3 (main, Feb 4 2025, 14:48:35) [GCC 13.3.0] on linux
- django-coverage-plugin==3.1.1
- coverage==7.10.1
In combination with:
- pytest==8.4.1
- pytest-cov==6.2.1
- pytest-django==4.11.1
- pytest-playwright==0.7.0