Skip to content
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

coverage gevent looks broken #149

Closed
nedbat opened this issue Sep 26, 2011 · 21 comments
Closed

coverage gevent looks broken #149

nedbat opened this issue Sep 26, 2011 · 21 comments
Labels
bug Something isn't working

Comments

@nedbat
Copy link
Owner

nedbat commented Sep 26, 2011

Originally reported by mardiros (Bitbucket: mardiros, GitHub: mardiros)


If you run the attachement test_gevent.py, coverage doesn't mark a few line as executed.


@nedbat
Copy link
Owner Author

nedbat commented Nov 18, 2011

Original comment by Anonymous


Confirmed, gevent breaks coverage :(

@nedbat
Copy link
Owner Author

nedbat commented Nov 9, 2012

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

@nedbat
Copy link
Owner Author

nedbat commented Nov 9, 2012

Here's a fix, though it sacrifices other behavior: https://github.com/newbrough/coverage

@nedbat
Copy link
Owner Author

nedbat commented Nov 9, 2012

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

@nedbat
Copy link
Owner Author

nedbat commented Feb 27, 2013

Original comment by krw1243 (Bitbucket: krw1243, GitHub: Unknown)


Hi Ned, wondering if you could describe what behavior the newbrough branch sacrifices for someone who doesn't have a deep understanding of coverage.py and gevent internals, so we can decide if this fix would be appropriate for our project (which uses gevent)?

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Original comment by Vadim Fint (Bitbucket: mocksoul, GitHub: mocksoul)


This monkey patch fixed coverage to work with gevent for me:

#!python
def patch_coverage_for_gevent():
    from coverage import collector as _collector, control as _control
    import collections as _collections
    import gevent as _gevent
    _PyTracer = _collector.PyTracer

    class DataStack(object):
        def __init__(self):
            self.__data = _collections.defaultdict(list)

        def __idx(self):
            return hash(_gevent.getcurrent())

        def pop(self):
            return self.__data[self.__idx()].pop()

        def append(self, value):
            return self.__data[self.__idx()].append(value)

    class PyTracer(_PyTracer):
        def __init__(self):
            _PyTracer.__init__(self)
            self.data_stack = DataStack()

    _collector.PyTracer = PyTracer

    # Patch coverage, so it uses timid simple tracer by default after monkey patching
    _coverage = _control.coverage

    class coverage(_coverage):
        def __init__(self, *args, **kwargs):
            kwargs.setdefault('timid', True)
            super(coverage, self).__init__(*args, **kwargs)

    __import__('pyjack').replace_all_refs(_coverage, coverage)

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Vadim, thanks, this will help as I evaluate solutions. Just one question: why did you use __import__ to import pyjack, instead of simply importing it?

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Original comment by Vadim Fint (Bitbucket: mocksoul, GitHub: mocksoul)


Nothing special, just to write replace_all_refs in one line :)

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Hmm, ok, strange what people value in code....

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Original comment by Vadim Fint (Bitbucket: mocksoul, GitHub: mocksoul)


"Import statement" at the end of meth looks ugly while
"import statement" on top will break all monkey patching if pyjack is not available

But anyway, this is more strange for me:

#!bash
(.venv)[trunk] mcsl .venv/lib/python2.7/site-packages/coverage % flake8 . | wc -l
352

You are right, it is strange what people value in code... :-

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Those are better reasons than "one line of code." :)

And it's even worse on my machine for some reason:

(coverage)~/coverage/trunk $ flake8 . | wc -l
    1874

@nedbat
Copy link
Owner Author

nedbat commented Oct 18, 2013

Original comment by Vadim Fint (Bitbucket: mocksoul, GitHub: mocksoul)


I have max line length tuned for pep8 checks (78 => 118 lines), probably this is the main reason

@nedbat
Copy link
Owner Author

nedbat commented Jul 5, 2014

Original comment by Brian Wylie (Bitbucket: brifordwylie, GitHub: brifordwylie)


The workbench project really needs gevent coverage https://github.com/SuperCowPowers/workbench. The project used to use nosetests and we pulled in https://github.com/newbrough/coverage.git. With that configuration we were getting about 94% coverage. We upgraded to pytest and the latest version complains/won't run with the github version of coverage above. So we switched to using the 'stock' coverage module and now we have a coveralls badge that is RED and kinda embarrassing :)

Would it be possible to contract someone to put in the support for gevent? Elance contract perhaps?

Happy to support this work because we'd really like to have our coverage badge not be red all the time, kinda erodes confidence in an embryonic project like ours.. :)

Cheers,
-bri

@nedbat
Copy link
Owner Author

nedbat commented Jul 6, 2014

@Brianwylie There is some nascent support for gevent in the current coverage.py code, but it needs a lot of love. I am happy to talk about possibilities for getting it done.

@nedbat
Copy link
Owner Author

nedbat commented Jul 14, 2014

@mocksoul I'm trying to adapt your solution, but getting odd results. Any chance you could help me understand what's going on?

@nedbat
Copy link
Owner Author

nedbat commented Jul 17, 2014

Original comment by Brian Wylie (Bitbucket: brifordwylie, GitHub: brifordwylie)


Hi Ned,

I also tried Vadim's patch and it didn't work for me either. Vadim said he wasn't surprised as these things go I think it was more a quick patch on a particular version than meant to be anything more long lasting. I ran the test 'test_gevent.py' above.. and after some random tinkering noticed that I would occasionally get a thread KeyError exception thrown at the very end (was playing with gevent monkey patching). I found this thread on SO http://stackoverflow.com/questions/8774958/keyerror-in-module-threading-after-a-successful-py-test-run. I think it might be a good place to start... to be honest it's deeper than my knowledge, so I'll just have to hope the pointer helps. :)

Replication Instructions:

  1. Add this as the first line of the script above

    from gevent import monkey; monkey.patch_all()

  2. Also note that newer versions of gevent no longer have the gevent.shutdown() call, so that can just be removed.

Just wanted to say that the workbench project ^appears^ to be getting exactly correct coverage with coveragepy 4.0a0 version (Github).. it's a complicated project (client/server) with tons of gevent calls (using ZeroRPC).. so that is a data point that perhaps the 4.0a0 version of coverage IS working on gevent calls, there's just some weird corner case at the very end of the program... perhaps with this thread KeyError exception getting thrown.

@nedbat
Copy link
Owner Author

nedbat commented Sep 18, 2014

Finally finished with 4ab31534f129 (bb), though more work to get it working with the C tracer.

@nedbat
Copy link
Owner Author

nedbat commented Sep 29, 2014

Original comment by Arnold Krille (Bitbucket: kampfschlaefer, GitHub: kampfschlaefer)


Sadly it seems to me as if its not working.

I created a small project to reproduce the issue: https://github.com/kampfschlaefer/gevent_covtest

It might be I am doing something wrong as I started using gevent only a week ago. But I don't think my code is wrong as it does what I want it to do...

@nedbat
Copy link
Owner Author

nedbat commented Sep 29, 2014

@kampfschlaefer There's no .coveragerc in that repo, so it looks to me like you haven't set the "concurrency" setting. Create a .coveragerc file like this:

[run]
concurrency = gevent

and try it again.

@nedbat
Copy link
Owner Author

nedbat commented Sep 29, 2014

Original comment by Arnold Krille (Bitbucket: kampfschlaefer, GitHub: kampfschlaefer)


Thanks, that did it!

@nedbat
Copy link
Owner Author

nedbat commented Sep 16, 2015

Original comment by Jason Madden (Bitbucket: jamadden, GitHub: jamadden)


I'm a maintainer of gevent itself, and I wanted to give a big shoutout to everyone who helped get this working. I've recently been able to enable this for gevent (here's the coveralls page) and it will be a big help to ensure gevent's quality moving forward. There are still some rough edges---some of the results aren't quite right---but I think that's more due to gevent's configuration and test system than anything else; overall, the coverage results make sense and look good!

@nedbat nedbat closed this as completed Sep 16, 2015
@nedbat nedbat added major bug Something isn't working labels Jun 23, 2018
This was referenced Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant