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

Django 1.7 model loading screws up test coverage for models #180

Open
diwu1989 opened this Issue Dec 21, 2014 · 14 comments

Comments

Projects
None yet
10 participants
@diwu1989
Copy link

diwu1989 commented Dec 21, 2014

Models are imported by the app configuration process
running ./manage.py test --with-coverage will report incorrect model coverage information because the models are all imported before the test command is executed

any way to fix this somehow?

@jchv

This comment has been minimized.

Copy link

jchv commented Dec 21, 2014

I've been trying to figure this out for the past couple days. My first guess would be that the system check module is interfering with this, but unfortunately setting DEBUG to False did not change anything. It appears you can do something like this, however:

$ coverage run ./manage.py test
$ coverage report
$ coverage html -d cover

to get things working with regular coverage. Unfortunately, this will require more manual setup, but at least it's possible to get things working this way. Right now, I don't think there's an easy fix to making coverage work inside of django-nose. :(

If you want to set things up this way, you'll probably want to make a .coveragerc file that, minimally, specifies source = . and an omit line for wsgi.py.

@diwu1989

This comment has been minimized.

Copy link
Author

diwu1989 commented Dec 26, 2014

Yes I figured out the more manual way of doing coverage worked as before, it's just sad to see that django nose's coverage feature is so broken in 1.7

@iyn

This comment has been minimized.

Copy link

iyn commented Apr 15, 2015

Still doesn't work with Django 1.8, need to run coverage manually. Is there a way to solve this?

this answer from coverage.py FAQ may be related (http://nedbatchelder.com/code/coverage/faq.html):

Q: Why do the bodies of functions (or classes) show as executed, but the def lines do not?

This happens because coverage is started after the functions are defined. The definition lines are executed without coverage measurement, then coverage is started, then the function is called. This means the body is measured, but the definition of the function itself is not.

To fix this, start coverage earlier. If you use the command line to run your program with coverage, then your entire program will be monitored. If you are using the API, you need to call coverage.start() before importing the modules that define your functions.

(emphasis mine)

@eliangcs

This comment has been minimized.

Copy link

eliangcs commented Apr 15, 2015

One way to work around it is to modify manage.py to call coverage.start() manually:

import os
import sys

if __name__ == "__main__":
    # ...
    from django.core.management import execute_from_command_line

    is_testing = 'test' in sys.argv

    if is_testing:
        import coverage
        cov = coverage.coverage(source=['package1', 'package2'], omit=['*/tests/*'])
        cov.erase()
        cov.start()

    execute_from_command_line(sys.argv)

    if is_testing:
        cov.stop()
        cov.save()
        cov.report()
@iyn

This comment has been minimized.

Copy link

iyn commented Apr 15, 2015

@eliangcs thanks for the workaround. 👍
I knew that it's possible to modify manage.py, although I don't consider it the "Django way", it would be better if this worked out of the box, not sure if that's possible though.

@kmmbvnr

This comment has been minimized.

Copy link

kmmbvnr commented Apr 29, 2015

If there is a way to initialize and start nose coverage in AppConfig.__init__ method, the coverage would be fine, if 'django-nose' would be first app in INSTALLED_APPS list.

That's how I fixed the coverage issue for django-jenkins - https://github.com/kmmbvnr/django-jenkins/blob/master/django_jenkins/apps.py

@jwhitlock

This comment has been minimized.

Copy link
Contributor

jwhitlock commented Jul 2, 2015

I'm not sure if there is a way to fix this purely in django-nose, although the AppConfig.__init__ solution sounds the closest. Does someone want to submit a doc PR with the three solutions mentioned?

jmcarp added a commit to jmcarp/tock that referenced this issue Jul 7, 2015

Start coverage before tests.
Django 1.7+ imports models before starting the testing process, so
models appear to never be tested. This patch uses a workaround described
in django-nose/django-nose#180.

[Resolves 18F#198]
@AkFede

This comment has been minimized.

Copy link

AkFede commented Sep 29, 2015

is there a solution in progress?

@jwhitlock

This comment has been minimized.

Copy link
Contributor

jwhitlock commented Oct 1, 2015

No, I don't believe there is a solution that will work for everyone. I've found @johnwchadwick's solution to be the most reliable, and I'm using that on my own projects.

@robguttman

This comment has been minimized.

Copy link

robguttman commented Oct 29, 2015

Any progress on a solution for this?

@jwhitlock

This comment has been minimized.

Copy link
Contributor

jwhitlock commented Oct 30, 2015

No, I don't believe there is a solution that will work for everyone. I've found @johnwchadwick's solution to be the most reliable, and I'm using that on my own projects.

@robguttman

This comment has been minimized.

Copy link

robguttman commented Oct 30, 2015

Deja vu! We use a number of nose plugins including for coverage (e.g., nosexcover for use with jenkins). So I would rather engage that existing nose coverage apparatus than try to repeat it all in a different stack. I tried messing around with the AppConfig.__init__ idea but it wasn't clear how to engage and start the same nose coverage worker that would get used downstream. Sigh - it's looking like I need to retool ..

vzvenyach pushed a commit to 18F/forecast that referenced this issue Nov 21, 2015

V David Zvenyach
Travis and coverage
Per django-nose/django-nose#180, I updated
the settings.py file to not import models before testing.

Also, updated the API for cloud.gov.

hugorodgerbrown added a commit to yunojuno-archive/django-package-monitor that referenced this issue Dec 6, 2015

Replace nose test runner with standard Django test runner
Getting coverage to cover everything alongside nose / django-nose was
proving too difficult - see this thread / comment for more details:
django-nose/django-nose#180 (comment)

I've removed the unused packages, and updated tox to use plain coverage
instead. The new test command is:

    coverage run --branch --include=package_monitor* manage.py test
    coverage report -m

If you want to see the HTML report, then run `coverage html`.
@reinout

This comment has been minimized.

Copy link

reinout commented Jun 30, 2016

In case someone is using buildout+djangorecipe: I just integrated @eliangcs 's solution (#180 (comment)) into djangorecipe. Works like a charm.

http://reinout.vanrees.org/weblog/2016/06/30/djangorecipe-test-coverage.html

@lfritts

This comment has been minimized.

Copy link

lfritts commented Sep 19, 2016

@eliangcs
Solution worked great...thanks for that!!

zunware added a commit to MapStory/mapstory that referenced this issue Jan 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment