nickstenning / honcho

Honcho: a python clone of Foreman. For managing Procfile-based applications.
http://pypi.python.org/pypi/honcho
MIT License
1.6k stars 146 forks source link

Fix the tox coverage target #106

Closed msabramo closed 9 years ago

msabramo commented 9 years ago

This change prevents the following error:

$ tox -e coverage
...
coverage runtests: commands[1] | nosetests --with-coverage --cover-branches --cover-package honcho honcho/test/unit
...........................E......................
======================================================================
ERROR: Failure: ImportError (cannot import name 'coverage')
----------------------------------------------------------------------
Traceback (most recent call last):
  ...
  File "/Users/marca/dev/git-repos/honcho/honcho/test/unit/test_manager.py", line 10, in <module>
    multiprocessing.Process = monkeypatch_process_for_coverage(multiprocessing.Process)
  File "/Users/marca/dev/git-repos/honcho/honcho/test/helpers.py", line 87, in monkeypatch_process_for_coverage
    from coverage.control import coverage
ImportError: cannot import name 'coverage'

It seems that in coverage==4.0a1 (which is somehow in my company's local devpi server but not on PyPI now; wonder if @nedbat pushed it to PyPI and then had second thoughts?), "coverage" (lowercase 'c') changed to "Coverage" (uppercase 'c'). So it seems wise to pin to coverage 3.x for now.

msabramo commented 9 years ago

Hmmm. coverage==4.0a1 is still on PyPI:

https://pypi.python.org/pypi/coverage/4.0a1

I was confused, because PyPI (even the page above) reports that 3.7.1 is the latest version. I think this is because 4.0a1 is a pre-release version and those are treated specially by PyPI and pip.

I think it's a different issue. I think the coverage target is failing in tox, because tox by default calls pip install --pre, so it will pull in pre-release packages like coverage==4.0a1. This is not happening in Travis CI, because pip install is not called with --pre.

That's the inconsistency I think.

Luckily, I think I saw that @hpk42 is thinking of removing --pre as a default in the next major version of tox...?

Anyway, the pinning I am doing here, is still a good idea I think. It will protect against failures when an official coverage==4.0.0 comes out...

nedbat commented 9 years ago

The error is because test/helpers.py is importing an internal name from coverage which changed in 4.0. The right import is from coverage import coverage, which still works. But the rest of that function is doing other things with internal names that might change. If you want supportable code, get in touch, and we can talk about how to do what you need to do without poking around in coverage internals.

nickstenning commented 9 years ago

@nedbat @msabramo This is unquestionably my fault. That horrible monkeypatch function is there to enable coverage across process boundaries. I have no doubt there is a better way to do this now. There may well even have been a better way to do this back when I added that monkeypatch. Ned?

hpk42 commented 9 years ago

for the record, tox-1.9 is going to default to not use "--pre" with pip.

nedbat commented 9 years ago

@nickstenning I don't know all the details of what you are facing, but coverage.py has ways to enable measurement of sub-processes: http://nedbatchelder.com/code/coverage/subprocess.html If that won't work for you, let's talk some more.

msabramo commented 9 years ago

I think this is not necessary now, because https://github.com/nickstenning/honcho/commit/dd159ba6a7b614c560e823275c319a386c35b00d has removed monkeypatch_process_for_coverage