Opened 9 years ago

Closed 9 years ago

#25209 closed Bug (fixed)

coverage html doesn't work as documented

Reported by: Peter Schmidt Owned by: nobody
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: Markus Holtermann, Shai Berger Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

After running the command, under OSX 10.10.4 / Python 3.4.2 (via homebrew) and coverage==3.7.1:

coverage run ./runtests.py

I get no .coverage file, after:
https://github.com/django/django/commit/4202959b6febd02cdaa712c666fa0d79569038ca

This appears to be caused by the parallel = True option, details:
http://nedbatchelder.com/code/coverage/config.html#h_run

For instance, I currently have:

(django_py3)pzrq@icebox:~/Projects/django/tests$ ls -a | grep .coverage
.coverage.icebox.local.42652.649933
.coverage.icebox.local.42665.263961
.coverage.icebox.local.42673.382480 
...
# Importantly no .coverage file

This means when you run downstream tools, such as coverage report -m or coverage html as recommended in the Django docs, i.e. https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/unit-tests/#code-coverage, you get the not very interesting output like the following:

(django_py3)pzrq@icebox:~/Projects/django/tests$ coverage report -m
Name    Stmts   Miss  Cover   Missing
-------------------------------------
(django_py3)pzrq@icebox:~/Projects/django/tests$ 

I can see two possible fixes to discuss:

  1. Remove parallel = True from .coveragerc
  2. Update the documentation such as https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/#code-coverage with the intermediate coverage combine step, to merge the different coverage report(s) together to generate a more typical .coverage file.

AFAIK multiprocessing isn't supported by the Django test runner, so +1 for option 1.

Change History (7)

comment:1 by Aymeric Augustin, 9 years ago

I'm working on a branch that allows parallel execution of Django tests with multiprocessing. It would be nice if it remained compatible with coverage measurement. Perhaps we can have different instructions for users of the --parallel option once it exists.

comment:2 by Tim Graham, 9 years ago

Cc: Markus Holtermann added
Triage Stage: UnreviewedAccepted

comment:3 by Peter Schmidt, 9 years ago

@aaugustin

TLDR My PR is currently removing one line. I'll happily put the coverage combine documentation and a revert of that one line removal (i.e. basically option 2) together tomorrow at PyConAU sprints if you think it will be useful for your parallel test runner.

But unless that's coming really soon it would be nice to restore the documentation to a more consistent state as Django does pride itself on documentation being a first class citizen.


https://github.com/django/django/compare/master...aaugustin:parallelize-tests-attempt-1
That looks really really cool!

I just wonder if how likely it is to make the Django 1.9 alpha around mid September 2015?
(Using as the guide Django 1.8 which took 2.5 months from alpha to release, and I fully appreciate that question may be impossible to answer until 1.9 alpha comes with or without it).

In any case it's a lot grander in scope than my attempt so far (discover-road-runner) - I usually use it dozens of times a day and it works wonders for my TDD productivity and testing other developers work more quickly (without needing --nomigrations in django-test-without-migrations); but it doesn't handle not-in-memory DBs, live application servers, file upload, etc. And I'd be the first to admit it's a kludge to generate test output cleanly and quickly (but has the benefit of not needing to pickle things like tracebacks if you print them straight away), and handle the really expensive migrations step better. With any luck OSX 10.11 El Capitan will mean you won't have to worry about all the headaches of the builtin multiprocessing module under OSX Python 2.7.6 at least (a colleague suggested I use the billiard fork entirely due to the time I have spent wrangling multiprocessing and sqlite3 crashes which AFAIK are fixed upstream and certainly in Python 3.4). I guess this is just a longer way of saying good luck - parallelism can be very challenging and fun!

comment:4 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

I think it's fine to commit this for now. Markus's project to improve coverage on Jenkins hasn't been completed and it's not clear to me that parallel=True should be the default..

comment:5 by Shai Berger, 9 years ago

Cc: Shai Berger added

FWIW, I promised to get Oracle support for the parallel test runner, and then couldn't really complete the work because of RL etc. I'll have more time for this after this weekend (when a conference I'm organizing is over).

comment:6 by Markus Holtermann, 9 years ago

I admit I might was a bit too eager to get parallel coverage support on Jenkins running but staled due to a missing feature in coverage: https://bitbucket.org/ned/coveragepy/issues/277/combined-parallel-report-should-use

comment:7 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In f6259ce7:

Fixed #25209 -- Removed parallel=True coverage option

Note: See TracTickets for help on using tickets.
Back to Top