pytest-dev/pytest-cov

Tests for template tags with pytest-xdist and pytest-cov break view tests using the template tags

TauPan opened this issue · 12 comments

TauPan commented

This was a weird one for me to figure out, but it should be simple to reproduce:

1.) You have a template tag library "foo"
2.) You have a view using a template using that library via load foo
3.) Being a thorough tester, you decide you need a test for the template tag library, which means you have to from app.templatetags import foo in your test.
4.) And of course you need to test the view using the templatetag.
5.) And maybe you have to test the templatetag before the view (not sure if this is relevant) e.g. pytest discovery puts it before the view test.
6.) And since you have many tests, you run pytest --cov -n 2

Which results in an error like the following:

django.template.exceptions.TemplateSyntaxError: 'foo' is not a registered tag library.

(maybe related to #110, but I don't see any of the problems described in that issue)

Workaround:

  • You put the business code you want to test in a different module (one that is not imported from a template) and register the tags manually via
from django import template
from ._foo import (tag1, tag2)

register = template.Library()

register.filter(is_safe=True)(tag1)
register.tag()(tag2)

And of course you import _foo in your test, instead of the template library.

I'm not sure if I should file this with pytest-xdist or pytest-cov, since it definitely only occurs if both are used. I'm filing this here first and we can figure out if it really is xdist's fault.

TauPan commented

I forgot to mention that tests even fail with -n 1.

ionelmc commented

Pretty sure you misdiagnosed this by forgetting to run the code that registers the tags. Tags aren't going to be magically available in the template, the code/module that registers the tags needs to be run/imported before your tests run the templates that use tags. Similar to how models aren't migrated if app ain't in INSTALLED_APPS.

TauPan commented

Pretty sure you misdiagnosed this by forgetting to run the code that registers the tags. Tags aren't going to be magically available in the template, the code/module that registers the tags needs to be run/imported before your tests run the templates that use tags. Similar to how models aren't migrated if app ain't in INSTALLED_APPS.

I'm pretty sure I didn't. See the code example. The tag library module initially looked something like this:

from django import template

register = template.Library()

@register.filter(is_safe=True)
def tag1(val):
  ...
@register.inclusion_tag('foo.html', takes_context=True)
def tag2(context):
  ...

Or where would you run the "code that registers the tags"? (In fact my first example was a bit wrong, as the template library in question only had filters and I tried to generalize it to tags without looking at the docs.)

Also I mentioned that this is only reproducible if both pytest-xdist and pytest-cov are used.

TauPan commented

Ok, this probably isn't very clear. I'll see if I can come up with a minimal project to reproduce this tomorrow

ionelmc commented

Ok, let me try to explain again.

If you got two xdist workers, A and B, then:

  • worker A runs:

    • test X which imports yourapp.templatetags and runs unrelated test
    • test Y which uses some tag from yourapp.templatetags and passes because the registry was loaded by unrelated test Y

  • worker B runs:

    • test Z which uses some tag from yourapp.templatetags and fails because the registry was loaded by side-effect from other tests

You'd think that scenario is unrealistic if it fails with -n 1 but then again, are you sure import side-effect doesn't come from collection? Eg, collection done in master process but actual test is done in in a worker process that doesn't do any test collection and subsequently doesn't have the necessary side-effect to pass.

TauPan commented

Ok, is was not that simple to reproduce and it turns out the error is only reproduced with django-coverage-plugin (which makes sense, since otherwise I'd have no coverage for templates).

This repository here: https://github.com/TauPan/pytest-django-xdist-cov-bug

Reproduces the problem with Django 1.11 (on current master, which is TauPan/pytest-django-xdist-cov-bug@4bfb956 ) and on Django 2.1 (at current master~1 which is the parent of the previous commit).

(I didn't anticipate that I would try out django 2.1 and 1.11, but since I could not reproduce with 2.1, I tried 1.11.)

Looks like the culprit might be https://github.com/nedbat/django_coverage_plugin/ but if you don't mind I'd like your opinion on this before I file another issue.

ionelmc commented

Can't say for sure until I try it but that plugin does try to import the django settings, and that could potentially be problematic. How are you setting up django? With pytest-django?

TauPan commented

Yup, with pytest-django. It's configured in the Pipfile.

Oops, there's a mistake in pytest.ini, settings module should be proj.settings.

ionelmc commented

@TauPan ok so is there still a bug? Also, how to run your example? I don't see reproduce instructions anywhere.

TauPan commented

@TauPan ok so is there still a bug? Also, how to run your example? I don't see reproduce instructions anywhere.

Sorry. To quote https://twitter.com/raganwald/status/466936669363793920:
"your README should always explain how to run your damn tests"

Added reproduction instructions to README.md. Hope that works for you.

ionelmc commented

Ok so I've tried the code. As explained before the problem are some missing imports. Apply this to fix your project:

diff --git a/.envrc b/.envrc
index 2188da1..990bc1d 100644
--- a/.envrc
+++ b/.envrc
@@ -1 +1,2 @@
 export DJANGO_SETTINGS_MODULE=proj.settings
+export PYTHONPATH=proj
diff --git a/proj/app/__init__.py b/proj/app/__init__.py
index e69de29..7ec32ee 100644
--- a/proj/app/__init__.py
+++ b/proj/app/__init__.py
@@ -0,0 +1 @@
+from . import templatetags
diff --git a/proj/app/templatetags/__init__.py b/proj/app/templatetags/__init__.py
index e69de29..2e046b5 100644
--- a/proj/app/templatetags/__init__.py
+++ b/proj/app/templatetags/__init__.py
@@ -0,0 +1 @@
+from . import tags
diff --git a/proj/pytest.ini b/proj/pytest.ini
deleted file mode 100644
index b0a6fc8..0000000
--- a/proj/pytest.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[pytest]
-# needs pytest-django:
-DJANGO_SETTINGS_MODULE = airshield.settings_test
  • 👍1
TauPan commented

Ok, it appears to be as you explained in #285 (comment)

in fact against current master the following patch is sufficient to not cause an error:

diff --git a/proj/app/__init__.py b/proj/app/__init__.py
index e69de29..7ec32ee 100644
--- a/proj/app/__init__.py
+++ b/proj/app/__init__.py
@@ -0,0 +1 @@
+from . import templatetags
diff --git a/proj/app/templatetags/__init__.py b/proj/app/templatetags/__init__.py
index e69de29..2e046b5 100644
--- a/proj/app/templatetags/__init__.py
+++ b/proj/app/templatetags/__init__.py
@@ -0,0 +1 @@
+from . import tags

pytest --cov -n2 completes green with the following result:

============================================================================ test session starts =============================================================================
platform linux -- Python 3.6.8, pytest-4.4.1, py-1.8.0, pluggy-0.9.0
Django settings: proj.settings (from environment variable)
rootdir: /home/friedel/git/pytest-django-xdist-cov-bug
plugins: xdist-1.28.0, forked-1.0.2, django-3.4.8, cov-2.7.1
gw0 [2] / gw1 [2]
..                                                                                                                                                                     [100%]

----------- coverage: platform linux, python 3.6.8-final-0 -----------
Name                                Stmts   Miss  Cover
-------------------------------------------------------
proj/app/__init__.py                    1      0   100%
proj/app/admin.py                       1      0   100%
proj/app/models.py                      1      0   100%
proj/app/templatetags/__init__.py       1      0   100%
proj/app/templatetags/tags.py           4      0   100%
proj/app/views.py                       3      0   100%
proj/proj/__init__.py                   0      0   100%
proj/proj/urls.py                       4      0   100%
-------------------------------------------------------
TOTAL                                  15      0   100%

========================================================================== 2 passed in 2.16 seconds ==========================================================================

But the result raises more questions (which is probably a good thing):

1.) home.html is not only reported with coverage of 0% (probably because of nedbat/django_coverage_plugin#36 ) but omitted completely.

2.) https://docs.djangoproject.com/en/2.2/howto/custom-template-tags/ (or 1.11 or any version) does not mention that templatetags libraries need to be imported anywhere. I'm not sure if any of the relevant modules mentions that (pytest-cov, pytest-xdist, pytest-django or django_coverage_plugin). Since production code runs without those imports (and --cov and -n2 on their own as well) I suspect there's still a bug somewhere and importing those modules explicitly is just a workaround, with the advantage that it's simpler than my initial workaround of moving the busines code of the template tags and filters out of the tag library module and testing it separately.

You seem sure that there's no bug in pytest-cov, so I suspect there might be one in pytest-django or pytest_coverage_plugin.

In any case, thanks for your assistance!