nedbat / coveragepy

The code coverage tool for Python
https://coverage.readthedocs.io
Apache License 2.0
3.03k stars 435 forks source link

Coverage fails with os.fork and os._exit #310

Open nedbat opened 10 years ago

nedbat commented 10 years ago

Originally reported by Marc Schlaich (Bitbucket: schlamar, GitHub: schlamar)


It is a common pattern to exit a child process with os._exit after a fork (ref). However, in this case coverage for the child process fails.

Example:

import os

def child():
    a = 1 + 1
    assert a == 2

def main():
    pid = os.fork()
    if pid:
        _, systemstatus = os.waitpid(pid, 0)
        assert not systemstatus
    else:
        child()
        os._exit(0)

if __name__ == '__main__':
    main()

Test run:

$ coverage run -p cov.py
$ coverage combine && coverage report -m
Name    Stmts   Miss  Cover   Missing
-------------------------------------
cov        13      4    69%   6-7, 16-17

nedbat commented 7 years ago

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@spaceone would you be so kind as to try to reproduce the issue with coverage 4.3.1 ? If you're still experiencing problems with that version I'll try to figure out why.

nedbat commented 7 years ago

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@spaceone the result I get (83% coverage) is with 4.3.1 and I confirm that I get exactly the same result as you with 4.2 (67% coverage), using python 2.7.12.

nedbat commented 7 years ago

Original comment by space one (Bitbucket: spaceone, GitHub: spaceone)


@dachary My example shows 67% coverage:

2016-12-20-134938_3840x1080_scrot.png

I use coverage 4.2 latest version from pip with python 2.7.9. How does it come that you have different results?

My example code is actually a stripped down version of our production code:

http://forge.univention.org/websvn/filedetails.php?repname=dev&path=%2Fbranches%2Fucs-4.2%2Fucs-4.2-0%2Fmanagement%2Funivention-directory-manager-modules%2Funivention-cli-client

http://forge.univention.org/websvn/filedetails.php?repname=dev&path=%2Fbranches%2Fucs-4.2%2Fucs-4.2-0%2Fmanagement%2Funivention-directory-manager-modules%2Funivention-cli-server

nedbat commented 7 years ago

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@spaceone when running the proposed reproducer I get the following ouptut:

#!bash

$ run.sh
bar()
foo()
Name     Stmts   Miss  Cover
----------------------------
foo.py       6      1    83%

The code it contains is more complicated than the reproducer provided in the description of this issue. It would be great if you could explain why and how it demonstrates something different.

nedbat commented 8 years ago

Original comment by space one (Bitbucket: spaceone, GitHub: spaceone)


@nedbat Here is an example. Extract the tar.gz and execute run.sh. This is basically what the components in our product do.

nedbat commented 8 years ago

@spaceone can you provide a reproducible example?

nedbat commented 8 years ago

Original comment by space one (Bitbucket: spaceone, GitHub: spaceone)


Any news on this? My application doesn't use pytest. (does it work with pytest?). I use os.fork() and the forked process doesn't write any results anymore. I monkey patched os.['execl', 'execle', 'execlp', 'execlpe', 'execv', 'execve', 'execvp', 'execvpe', 'fork', '_exit'] similar to https://bitbucket.org/ned/coveragepy/issues/43/coverage-measurement-fails-on-code but this only writes the results prior to forking.

nedbat commented 10 years ago

Original comment by Marc Schlaich (Bitbucket: schlamar, GitHub: schlamar)


@rstuart85 As already said, I have no idea how to implicitly support os._exit. pytest-cov supports py.process.ForkedFunc (which is a wrapper around fork/os._exit) by using explicit before and after process hooks.

However, in theory it should work with sys.exit. If that's not working, please report it at https://github.com/schlamar/pytest-cov with a test case. Thanks!

nedbat commented 10 years ago

Original comment by Ryan Stuart (Bitbucket: rstuart85, GitHub: rstuart85)


@schlamar Just to be clear, the problem of it not capturing lines seems to be fixed but it certainly doesn't capture the lines in the subprocesses no matter if I exit with os._exit(), sys.exit() or my hack above.

nedbat commented 10 years ago

Original comment by Ryan Stuart (Bitbucket: rstuart85, GitHub: rstuart85)


@schlamar It isn't fixed for me.

coverage run --source my_dir -p -m py.test my_dir returns 100%

py.test --cov my_dir my_dir returns 95%

Where is the appropriate place to take this discussion?

nedbat commented 10 years ago

Original comment by Marc Schlaich (Bitbucket: schlamar, GitHub: schlamar)


@rstuart85 This is fixed in pytest-cov 1.7.0 (https://github.com/schlamar/pytest-cov/pull/4), please give it a try! (This is even mentioned in the linked SO answer if you would have fully read it...)

@nedbat Yes, that would work: https://github.com/schlamar/pytest-cov/commit/fb100dba36079227a3f491d3d055f9e377031c59 :) However, I wouldn't dare to put something like this in production...

nedbat commented 10 years ago

Hmm, we have a few ideas here, let's not close this just yet.

And somehow, no one has suggested monkey-patching os._exit yet.

nedbat commented 10 years ago

Original comment by Ryan Stuart (Bitbucket: rstuart85, GitHub: rstuart85)


@schlamar if you want to acheive 100% test coverage then, AFAIK, pytest-cov is not a workable solution. It gets invoked too late to capture all lines. See http://stackoverflow.com/questions/16404716/using-py-test-with-coverage-doesnt-include-imports/16524426#16524426 for example.

nedbat commented 10 years ago

Original comment by Marc Schlaich (Bitbucket: schlamar, GitHub: schlamar)


There is no other way than explicitly calling stop + save before os._exit. It is not possible to handle this automatically so I'm going to close this issue.

@rstuart85 pytest-cov can handle py.process.ForkedFunc since py's latest release yesterday. Relevant commits are:

nedbat commented 10 years ago

Original comment by Ryan Stuart (Bitbucket: rstuart85, GitHub: rstuart85)


I don't actually want to use that code, its just the only way I can get coverage and py.test with os._exit to work properly.

Cheers

nedbat commented 10 years ago

@schlamar The method you want is currently called coverage._atexit. I could rename that to coverage.shutdown. Try using coverage._atexit() now and tell me if it works well. Or you could use coverage.stop(); coverage.save().

nedbat commented 10 years ago

@rstuart85 If I read this code right, it's a way to get access to the coverage instance (if any), and then calling it explicitly from the product code. If the product code can be changed to accommodate this problem, then there's lots of possibilities, but people generally don't want to do that.

nedbat commented 10 years ago

Original comment by Ryan Stuart (Bitbucket: rstuart85, GitHub: rstuart85)


Here is a dirty hack to work around this issue:

#!python
    def _coverage_hack(cls):
        """God awful hack to make coverage and py.test work together."""
        class Cov(object):
            stop = lambda self: None
            save = lambda self: None
        cov = Cov()
        if "coverage" in sys.modules:
            import coverage
            try:
                raise ZeroDivisionError
            except ZeroDivisionError:
                f = sys.exc_info()[2].tb_frame
            tb = []
            while f:
                tb.append(f)
                f = f.f_back
            t = tb[-3]
            if 'self' in t.f_locals:
                slf = t.f_locals['self']
                if hasattr(slf, "coverage"):
                    if isinstance(slf.coverage, coverage.coverage):
                        cov = slf.coverage
        return cov
    _coverage_hack = classmethod(_coverage_hack)

It can be used as follows:

#!python

self.__coverage = self._coverage_hack()
child_pid = os.fork()
if child_pid == 0:
    # Do work
    ((self.__coverage.stop(),) and (self.__coverage.save(),) and os._exit(0))
nedbat commented 10 years ago

Issue #312 was marked as a duplicate of this issue.

nedbat commented 10 years ago

Original comment by Marc Schlaich (Bitbucket: schlamar, GitHub: schlamar)


Your test run data is incorrect, it actually is:

Bitbucket has stripped some new lines ;)

I'm not sure what I can do to fix this. os._exit skips any further work in the process

Yes, I feared this cannot be handled automatically. Anyway, it would be enough if I can do the clean up manually before os._exit but AFAIS there is no useful entry point in the API. Something like coverage.shutdown would be nice.

nedbat commented 10 years ago

Your test run data is incorrect, it actually is:

Name     Stmts   Miss  Cover   Missing
--------------------------------------
bug310      13      4    69%   4-5, 14-15

I'm not sure what I can do to fix this. os._exit skips any further work in the process, including the work coverage.py needs to do to write out its measured data.

jorgeecardona commented 3 years ago

I am not sure why is this "exotic". Using os._exit is the recommended way to exit from a forked child.

Anyway, I workaround this by simple patching the os._exit function using unittest.mock.patch and coverage.Coverage.current like this:

def test_os_fork(self):
    _os_exit = os._exit

    def _exit(status):
        cov = coverage.Coverage.current()
        cov.stop()
        cov.save()
        _os_exit(status)

    with patch('mypackage.os._exit', new=_exit):
        mypackage.function_that_uses_os_fork()

The resulting reports can be combined as usual after that.

nedbat commented 3 years ago

"exotic" wasn't intended to mean "low-priority", but "needs research into a thing I am not familiar with." Thanks for sharing a workaround.