More coverage (#1085)

* Massively simplify the internal _threading.py by removing everything we don't use.

Enable coverage testing for Python 3 and some for PyPy.

* PyPy can't handle coverage :(

* Account for a racy test and tweak the makefile to ignore errors uploading to coveralls. Python 3.7 was seen to generate that: https://travis-ci.org/gevent/gevent/jobs/334831358

* Manually exclude .so files. They caused coveralls to fail: https://travis-ci.org/gevent/gevent/jobs/334839765#L1755

* Enable coverage on pypy too.

* coverage pragmas

* Skip a test that fails under coverage sometimes

.travis.yml

@@ -10,7 +10,7 @@ env:
     matrix:
       # These are ordered to get as much diversity in the
       # first group of parallel runs (4) as posible
-      - TASK=lint-py27
+      - TASK=test-py27
       - TASK=test-pypy
       - TASK=test-py36
       - TASK=test-py37

CHANGES.rst

@@ -23,6 +23,9 @@
   They have always had the same default as Python 3, namely an empty
   tuple and false, but now are accessible to Python 2.
 
+- The internal, undocumented module ``gevent._threading`` has been
+  simplified.
+
 
 1.3a1 (2018-01-27)
 ==================

Makefile

@@ -17,9 +17,9 @@ clean:
 	rm -f src/gevent/ares.c src/gevent/ares.h
 	rm -f src/gevent/_semaphore.c src/gevent/_semaphore.h
 	rm -f src/gevent/local.c src/gevent/local.h
-	rm -f src/gevent/*.so src/gevent/libev/*.so src/gevent/libuv/*.so
+	rm -f src/gevent/*.so src/gevent/*.pyd src/gevent/libev/*.so src/gevent/libuv/*.so src/gevent/libev/*.pyd src/gevent/libuv/*.pyd
 	rm -rf src/gevent/libev/*.o src/gevent/libuv/*.o src/gevent/*.o
-	rm -rf src/gevent/__pycache__ src/greentest/__pycache__ src/gevent/libev/__pycache__
+	rm -rf src/gevent/__pycache__ src/greentest/__pycache__ src/greentest/greentest/__pycache__ src/gevent/libev/__pycache__
 	rm -rf src/gevent/*.pyc src/greentest/*.pyc src/gevent/libev/*.pyc
 	rm -rf src/greentest/htmlcov src/greentest/.coverage
 	rm -rf build
@@ -80,13 +80,17 @@ threadfiletest:
 
 allbackendtest:
 	${PYTHON} scripts/travis.py fold_start default "Testing default backend"
-	GEVENT_CORE_CFFI_ONLY= make alltest
+	GEVENT_CORE_CFFI_ONLY= GEVENTTEST_COVERAGE=1 make alltest
 	${PYTHON} scripts/travis.py fold_end default
-	make cffibackendtest
+	GEVENTTEST_COVERAGE=1 make cffibackendtest
+# because we set parallel=true, each run produces new and different coverage files; they all need
+# to be combined
+	make coverage_combine
+
 
 cffibackendtest:
 	${PYTHON} scripts/travis.py fold_start libuv "Testing libuv backend"
-	GEVENT_CORE_CFFI_ONLY=libuv make alltest
+	GEVENT_CORE_CFFI_ONLY=libuv GEVENTTEST_COVERAGE=1 make alltest
 	${PYTHON} scripts/travis.py fold_end libuv
 	${PYTHON} scripts/travis.py fold_start libev "Testing libev CFFI backend"
 	GEVENT_CORE_CFFI_ONLY=libev make alltest
@@ -100,16 +104,15 @@ leaktest: test_prelim
 bench:
 	${PYTHON} src/greentest/bench_sendall.py
 
-
 travis_test_linters:
 	make lint
-	GEVENTTEST_COVERAGE=1 make leaktest
-	GEVENTTEST_COVERAGE=1 make cffibackendtest
-# because we set parallel=true, each run produces new and different coverage files; they all need
-# to be combined
+	make leaktest
+	make cffibackendtest
+
+coverage_combine:
 	coverage combine . src/greentest/
 
-	coveralls --rcfile=src/greentest/.coveragerc
+	-coveralls --rcfile=src/greentest/.coveragerc
 
 
 .PHONY: clean doc prospector lint travistest travis
@@ -176,11 +179,8 @@ develop:
 	GEVENTSETUP_EV_VERIFY=3 python -m pip install -U -r dev-requirements.txt
 	${PYTHON} scripts/travis.py fold_end install
 
-lint-py27: $(PY27)
-	PYTHON=python2.7.14 PATH=$(BUILD_RUNTIMES)/versions/python2.7.14/bin:$(PATH) make develop travis_test_linters
-
 test-py27: $(PY27)
-	PYTHON=python2.7.14 PATH=$(BUILD_RUNTIMES)/versions/python2.7.14/bin:$(PATH) make develop allbackendtest
+	PYTHON=python2.7.14 PATH=$(BUILD_RUNTIMES)/versions/python2.7.14/bin:$(PATH) make develop lint leaktest allbackendtest
 
 test-py34: $(PY34)
 	PYTHON=python3.4.7 PATH=$(BUILD_RUNTIMES)/versions/python3.4.7/bin:$(PATH) make develop allbackendtest
@@ -195,7 +195,7 @@ test-py37: $(PY37)
 	PYTHON=python3.7.0a3 PATH=$(BUILD_RUNTIMES)/versions/python3.7.0a3/bin:$(PATH) make develop allbackendtest
 
 test-pypy: $(PYPY)
-	PYTHON=$(PYPY) PATH=$(BUILD_RUNTIMES)/versions/pypy590/bin:$(PATH) make develop cffibackendtest
+	PYTHON=$(PYPY) PATH=$(BUILD_RUNTIMES)/versions/pypy590/bin:$(PATH) make develop cffibackendtest coverage_combine
 
 test-pypy3: $(PYPY3)
 	PYTHON=$(PYPY3) PATH=$(BUILD_RUNTIMES)/versions/pypy3.5_590/bin:$(PATH) make develop basictest

src/gevent/_ffi/watcher.py

@@ -103,13 +103,6 @@ def only_if_watcher(func):
         return _NoWatcherResult
     return if_w
 
-def error_if_no_watcher(func):
-    @functools.wraps(func)
-    def no_w(self):
-        if not self._watcher:
-            raise ValueError("No watcher present", self)
-        func(self)
-    return no_w
 
 class LazyOnClass(object):
 
@@ -122,7 +115,7 @@ class LazyOnClass(object):
         self.func = func
 
     def __get__(self, inst, klass):
-        if inst is None:
+        if inst is None: # pragma: no cover
             return self
 
         val = self.func(inst)
@@ -147,7 +140,7 @@ class AbstractWatcherType(type):
     def __new__(cls, name, bases, cls_dict):
         if name != 'watcher' and not cls_dict.get('_watcher_skip_ffi'):
             cls._fill_watcher(name, bases, cls_dict)
-        if '__del__' in cls_dict and not ALLOW_WATCHER_DEL:
+        if '__del__' in cls_dict and not ALLOW_WATCHER_DEL: # pragma: no cover
             raise TypeError("CFFI watchers are not allowed to have __del__")
         return type.__new__(cls, name, bases, cls_dict)
 
@@ -166,7 +159,7 @@ class AbstractWatcherType(type):
                     return getattr(b, attr)
                 except AttributeError:
                     continue
-            if error:
+            if error: # pragma: no cover
                 raise AttributeError(attr)
         _watcher_prefix = cls_dict.get('_watcher_prefix') or _mro_get('_watcher_prefix', bases)
 

src/gevent/_socket3.py

@@ -665,7 +665,7 @@ if hasattr(_socket, "socketpair"):
         b = socket(family, type, proto, b.detach())
         return a, b
 
-else:
+else: # pragma: no cover
     # Origin: https://gist.github.com/4325783, by Geert Jansen.  Public domain.
 
     # gevent: taken from 3.6 release. Expected to be used only on Win. Added to Win/3.5
@@ -1035,8 +1035,10 @@ class _basefileobject(object):
 
 try:
     from gevent.fileobject import FileObjectPosix
-except ImportError:
-    # Manual implementation
+except ImportError: # pragma: no cover
+    # Manual implementation, only on Windows
+    # XXX: I think we could simplify this using FileObjectCommon
+    # and just implementing the IOBase interface?
     _fileobject = _basefileobject
 else:
     class _fileobject(FileObjectPosix):

src/gevent/libuv/corecffi.py

-from __future__ import absolute_import, print_function
-
-
-__all__ = [
-
-]
-
-import gevent.libuv._corecffi as _corecffi # pylint:disable=no-name-in-module,import-error
-
-ffi = _corecffi.ffi  # pylint:disable=no-member
-libuv = _corecffi.lib  # pylint:disable=no-member

src/gevent/libuv/watcher.py

@@ -135,10 +135,6 @@ class watcher(_base.watcher):
         _dbg("Creating", type(self), "with ref", ref)
         self.ref = ref
 
-    def _watcher_ffi_set_priority(self, priority):
-        # libuv has no concept of priority
-        pass
-
     def _watcher_ffi_init(self, args):
         # TODO: we could do a better job chokepointing this
         return self._watcher_init(self.loop.ptr,

src/gevent/local.py

@@ -462,7 +462,7 @@ try:
         local.__new__ = __new__
     else:
         local.__new__ = classmethod(__new__)
-except TypeError:
+except TypeError: # pragma: no cover
     pass
 finally:
     del sys

src/gevent/timeout.py

@@ -36,8 +36,7 @@ class _FakeTimer(object):
     def stop(self):
         return
 
-    def cancel(self):
-        return
+    cancel = stop
 
     stop = close = cancel
 

src/greentest/.coveragerc

@@ -3,15 +3,28 @@
 # concurrency=greenlet, except it causes coverage itself to import
 # gevent. That messes up our coverage numbers for top-level
 # statements, so we use greenlet instead. See https://github.com/gevent/gevent/pull/655#issuecomment-141198002
+# See also .coveragerc-pypy
 concurrency = greenlet
 parallel = True
 source = gevent
-omit = test_*
+omit =
+    # This is for <= 2.7.8, which we don't test
+    src/gevent/_ssl2.py
+    src/gevent/libev/_corecffi_build.py
+    src/gevent/libuv/_corecffi_build.py
+	src/gevent/win32util.py
+    # having concurrency=greenlet means that the Queue class
+    # which is used from multiple real threads doesn't
+    # properly get covered.
+	src/gevent/_threading.py
+    test_*
+    # local.so sometimes gets included, and it can't be parsed
+    # as source, so it fails the whole process.
+    *.so
+
 
 [report]
-# Coverage is run on Linux under cPython 2, so
-# exclude branches that are windows specific or pypy/python3
-# specific
+# Coverage is run on Linux under cPython 2/3 and pypy
 exclude_lines =
     pragma: no cover
     def __repr__
@@ -19,9 +32,6 @@ exclude_lines =
     raise NotImplementedError
     except ImportError:
     if __name__ == .__main__.:
-    if PYPY:
-    if PY3:
     if sys.platform == 'win32':
     if mswindows:
     if is_windows:
-    if sys.version_info.*>=.*3

src/greentest/.coveragerc-pypy

+[run]
+# This is just like .coveragerc, but
+# used for PyPy running. pypy doesn't support concurrency=greenlet
+parallel = True
+source = gevent
+omit =
+    # This is for <= 2.7.8, which we don't test
+    src/gevent/_ssl2.py
+    src/gevent/libev/_corecffi_build.py
+    src/gevent/libuv/_corecffi_build.py
+	src/gevent/win32util.py
+    # having concurrency=greenlet means that the Queue class
+    # which is used from multiple real threads doesn't
+    # properly get covered.
+	src/gevent/_threading.py
+    test_*
+    # local.so sometimes gets included, and it can't be parsed
+    # as source, so it fails the whole process.
+    *.so
+
+
+[report]
+# Coverage is run on Linux under cPython 2/3 and pypy, so
+# exclude branches that are windows specific or pypy
+# specific
+exclude_lines =
+    pragma: no cover
+    def __repr__
+    raise AssertionError
+    raise NotImplementedError
+    except ImportError:
+    if __name__ == .__main__.:
+    if sys.platform == 'win32':
+    if mswindows:
+    if is_windows:

src/greentest/coveragesite/sitecustomize.py

@@ -2,4 +2,16 @@
 # on the path as per https://coverage.readthedocs.io/en/coverage-4.0b3/subprocess.html.
 # Note that this disables other sitecustomize.py files.
 import coverage
-coverage.process_startup()
+try:
+    coverage.process_startup()
+except coverage.CoverageException as e:
+    if str(e) == "Can't support concurrency=greenlet with PyTracer, only threads are supported":
+        pass
+    else:
+        import traceback
+        traceback.print_exc()
+        raise
+except:
+    import traceback
+    traceback.print_exc()
+    raise

src/greentest/greentest/skipping.py

@@ -66,6 +66,8 @@ if sysinfo.PYPY3:
 else:
     skipOnPyPy3 = _do_not_skip
 
+skipUnderCoverage = unittest.skip if sysinfo.RUN_COVERAGE else _do_not_skip
+
 skipIf = unittest.skipIf
 
 

src/greentest/greentest/testrunner.py

@@ -13,6 +13,7 @@ from multiprocessing import cpu_count
 from greentest import util
 from greentest.util import log
 from greentest.sysinfo import RUNNING_ON_CI
+from greentest.sysinfo import PYPY
 from greentest import six
 
 
@@ -37,11 +38,22 @@ RUN_ALONE = [
 IGNORE_COVERAGE = [
     # Hangs forever
     'test__threading_vs_settrace.py',
+    # times out
+    'test_socket.py',
+    # Doesn't get the exceptions it expects
+    'test_selectors.py',
     # XXX ?
     'test__issue302monkey.py',
     "test_subprocess.py",
 ]
 
+if PYPY:
+    IGNORE_COVERAGE += [
+        # Tends to timeout
+        'test__refcount.py',
+        'test__greenletset.py'
+    ]
+
 
 def run_many(tests, expected=(), failfast=False, quiet=False):
     # pylint:disable=too-many-locals
@@ -283,6 +295,8 @@ def main():
         coverage = True
         # NOTE: This must be run from the greentest directory
         os.environ['COVERAGE_PROCESS_START'] = os.path.abspath(".coveragerc")
+        if PYPY:
+            os.environ['COVERAGE_PROCESS_START'] = os.path.abspath(".coveragerc-pypy")
         os.environ['PYTHONPATH'] = os.path.abspath("coveragesite") + os.pathsep + os.environ.get("PYTHONPATH", "")
         # We change directory often, use an absolute path to keep all the
         # coverage files (which will have distinct suffixes because of parallel=true in .coveragerc

src/greentest/test__event.py

 from __future__ import absolute_import, print_function, division
-import greentest
+
 import gevent
 from gevent.event import Event, AsyncResult
+
+import greentest
+from greentest.skipping import skipUnderCoverage
 from greentest.six import xrange
 
 DELAY = 0.01
@@ -100,28 +103,31 @@ class TestAsyncResult(greentest.TestCase):
         gevent.sleep(0)
         self.assertEqual(log, [('caught', obj)])
 
+    @skipUnderCoverage("This test is racy and sometimes fails")
     def test_set(self):
         event1 = AsyncResult()
-        event2 = AsyncResult()
-
         timer_exc = MyException('interrupted')
 
-        g = gevent.spawn_later(DELAY / 2.0, event1.set, 'hello event1')
+        # Notice that this test is racy
+        g = gevent.spawn_later(DELAY, event1.set, 'hello event1')
         t = gevent.Timeout.start_new(0, timer_exc)
         try:
             with self.assertRaises(MyException) as exc:
                 event1.get()
             self.assertEqual(timer_exc, exc.exception)
-
-            X = object()
-            result = gevent.with_timeout(DELAY, event2.get, timeout_value=X)
-            self.assertIs(
-                result, X,
-                'Nobody sent anything to event2 yet it received %r' % (result, ))
         finally:
             t.close()
             g.kill()
 
+    def test_set_with_timeout(self):
+        event2 = AsyncResult()
+
+        X = object()
+        result = gevent.with_timeout(DELAY, event2.get, timeout_value=X)
+        self.assertIs(
+            result, X,
+            'Nobody sent anything to event2 yet it received %r' % (result, ))
+
     def test_nonblocking_get(self):
         ar = AsyncResult()
         self.assertRaises(gevent.Timeout, ar.get, block=False)