Minor cleanup of test__os

We saw a Windows crash (https://ci.appveyor.com/project/denik/gevent/builds/20295148/job/ldltku6gletkdfgl):

| C:\Python37-x64\python.exe -u -mgevent.tests.test__os
461  .
462  ----------------------------------------------------------------------
463  Ran 1 test in 1.656s
464
465  OK
466  Windows fatal exception: access violation
467
468  Thread 0x000003dc (most recent call first):
469
470  Current thread 0x000005c8 (most recent call first):
471    File "C:\Python37-x64\lib\site-packages\gevent\threadpool.py", line 261 in _worker
472
473  Thread 0x00000af8 (most recent call first):
474    File "C:\Python37-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
475    File "C:\Python37-x64\lib\site-packages\gevent\_threading.py", line 166 in get
476    File "C:\Python37-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
477
478  Thread 0x00000424 (most recent call first):
479    File "C:\Python37-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
480    File "C:\Python37-x64\lib\site-packages\gevent\_threading.py", line 166 in get
481    File "C:\Python37-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
482
483  Thread 0x000003dc (most recent call first):
484    File "C:\Python37-x64\lib\weakref.py", line 356 in remove
485
486! C:\Python37-x64\python.exe -u -mgevent.tests.test__os [code 3221225477] [took 1.7s]

I confirmed that the one test we run there doesn't create an
IdentRegistry. Instead, the WeakKeyDictionary that's being created
comes from either zope.interface (tracking dependents) or
concurrent.futures (tracking spawned threads).

src/gevent/testing/__init__.py

@@ -68,6 +68,7 @@ from .skipping import skipOnPyPy
 from .skipping import skipOnPyPyOnCI
 from .skipping import skipOnPyPy3
 from .skipping import skipIf
+from .skipping import skipUnless
 from .skipping import skipOnLibev
 from .skipping import skipOnLibuv
 from .skipping import skipOnLibuvOnWin

src/gevent/testing/skipping.py

@@ -92,6 +92,7 @@ if sysinfo.PYPY:
 skipUnderCoverage = unittest.skip if sysinfo.RUN_COVERAGE else _do_not_skip
 
 skipIf = unittest.skipIf
+skipUnless = unittest.skipUnless
 
 
 

src/gevent/tests/test__hub.py

@@ -36,8 +36,6 @@ DELAY = 0.1
 class TestCloseSocketWhilePolling(greentest.TestCase):
 
     def test(self):
-        from gevent.exceptions import ConcurrentObjectUseError
-
         sock = socket.socket()
         self._close_on_teardown(sock)
         t = get_hub().loop.timer(0)

src/gevent/tests/test__os.py

@@ -6,17 +6,17 @@ from os import pipe
 
 import gevent
 from gevent import os
-from gevent.testing import TestCase, main, LARGE_TIMEOUT
 from gevent import Greenlet, joinall
 
+from gevent import testing as greentest
 from gevent.testing import mock
 from gevent.testing import six
 from gevent.testing.skipping import skipOnLibuvOnPyPyOnWin
 
 
-class TestOS_tp(TestCase):
+class TestOS_tp(greentest.TestCase):
 
-    __timeout__ = LARGE_TIMEOUT
+    __timeout__ = greentest.LARGE_TIMEOUT
 
     def pipe(self):
         return pipe()
@@ -53,8 +53,8 @@ class TestOS_tp(TestCase):
         # the pipe before the consumer starts, and would block the entire
         # process. Therefore the next line would never finish.
         joinall([producer, consumer])
-        assert bytesread[0] == nbytes
-        assert bytesread[0] == byteswritten[0]
+        self.assertEqual(bytesread[0], nbytes)
+        self.assertEqual(bytesread[0], byteswritten[0])
 
     if sys.version_info[0] < 3:
 
@@ -67,108 +67,112 @@ class TestOS_tp(TestCase):
             self._test_if_pipe_blocks(six.builtins.memoryview)
 
 
-if hasattr(os, 'make_nonblocking'):
+@greentest.skipUnless(hasattr(os, 'make_nonblocking'),
+                      "Only on POSIX")
+class TestOS_nb(TestOS_tp):
 
-    class TestOS_nb(TestOS_tp):
+    def read(self, fd, count):
+        return os.nb_read(fd, count)
 
-        read = staticmethod(os.nb_read)
-        write = staticmethod(os.nb_write)
+    def write(self, fd, count):
+        return os.nb_write(fd, count)
 
-        def pipe(self):
-            r, w = super(TestOS_nb, self).pipe()
-            os.make_nonblocking(r)
-            os.make_nonblocking(w)
-            return r, w
-
-        def _make_ignored_oserror(self):
-            import errno
-            ignored_oserror = OSError()
-            ignored_oserror.errno = errno.EINTR
-            return ignored_oserror
-
-
-        def _check_hub_event_closed(self, mock_get_hub, fd, event):
-            mock_get_hub.assert_called_once_with()
-            hub = mock_get_hub.return_value
-            io = hub.loop.io
-            io.assert_called_once_with(fd, event)
-
-            event = io.return_value
-            event.close.assert_called_once_with()
-
-        def _test_event_closed_on_normal_io(self, nb_func, nb_arg,
-                                            mock_io, mock_get_hub, event):
-            mock_io.side_effect = [self._make_ignored_oserror(), 42]
-
-            fd = 100
-            result = nb_func(fd, nb_arg)
-            self.assertEqual(result, 42)
-
-            self._check_hub_event_closed(mock_get_hub, fd, event)
-
-        def _test_event_closed_on_io_error(self, nb_func, nb_arg,
-                                           mock_io, mock_get_hub, event):
-            mock_io.side_effect = [self._make_ignored_oserror(), ValueError()]
-
-            fd = 100
-
-            with self.assertRaises(ValueError):
-                nb_func(fd, nb_arg)
-
-            self._check_hub_event_closed(mock_get_hub, fd, event)
-
-        @mock.patch('gevent.os.get_hub')
-        @mock.patch('gevent.os._write')
-        def test_event_closed_on_write(self, mock_write, mock_get_hub):
-            self._test_event_closed_on_normal_io(os.nb_write, b'buf',
-                                                 mock_write, mock_get_hub,
-                                                 2)
-
-        @mock.patch('gevent.os.get_hub')
-        @mock.patch('gevent.os._write')
-        def test_event_closed_on_write_error(self, mock_write, mock_get_hub):
-            self._test_event_closed_on_io_error(os.nb_write, b'buf',
-                                                mock_write, mock_get_hub,
-                                                2)
-
-        @mock.patch('gevent.os.get_hub')
-        @mock.patch('gevent.os._read')
-        def test_event_closed_on_read(self, mock_read, mock_get_hub):
-            self._test_event_closed_on_normal_io(os.nb_read, b'buf',
-                                                 mock_read, mock_get_hub,
-                                                 1)
-
-        @mock.patch('gevent.os.get_hub')
-        @mock.patch('gevent.os._read')
-        def test_event_closed_on_read_error(self, mock_read, mock_get_hub):
-            self._test_event_closed_on_io_error(os.nb_read, b'buf',
-                                                mock_read, mock_get_hub,
-                                                1)
-
-
-if hasattr(os, 'fork_and_watch'):
-
-    class TestForkAndWatch(TestCase):
-
-        __timeout__ = LARGE_TIMEOUT
-
-        def test_waitpid_all(self):
-            # Cover this specific case.
-            pid = os.fork_and_watch()
-            if pid:
-                os.waitpid(-1, 0)
-                # Can't assert on what the pid actually was,
-                # our testrunner may have spawned multiple children.
-                os._reap_children(0) # make the leakchecker happy
-            else: # pragma: no cover
-                gevent.sleep(2)
-                os._exit(0)
-
-        def test_waitpid_wrong_neg(self):
-            self.assertRaises(OSError, os.waitpid, -2, 0)
+    def pipe(self):
+        r, w = super(TestOS_nb, self).pipe()
+        os.make_nonblocking(r)
+        os.make_nonblocking(w)
+        return r, w
+
+    def _make_ignored_oserror(self):
+        import errno
+        ignored_oserror = OSError()
+        ignored_oserror.errno = errno.EINTR
+        return ignored_oserror
+
+
+    def _check_hub_event_closed(self, mock_get_hub, fd, event):
+        mock_get_hub.assert_called_once_with()
+        hub = mock_get_hub.return_value
+        io = hub.loop.io
+        io.assert_called_once_with(fd, event)
+
+        event = io.return_value
+        event.close.assert_called_once_with()
+
+    def _test_event_closed_on_normal_io(self, nb_func, nb_arg,
+                                        mock_io, mock_get_hub, event):
+        mock_io.side_effect = [self._make_ignored_oserror(), 42]
+
+        fd = 100
+        result = nb_func(fd, nb_arg)
+        self.assertEqual(result, 42)
+
+        self._check_hub_event_closed(mock_get_hub, fd, event)
+
+    def _test_event_closed_on_io_error(self, nb_func, nb_arg,
+                                       mock_io, mock_get_hub, event):
+        mock_io.side_effect = [self._make_ignored_oserror(), ValueError()]
+
+        fd = 100
+
+        with self.assertRaises(ValueError):
+            nb_func(fd, nb_arg)
+
+        self._check_hub_event_closed(mock_get_hub, fd, event)
+
+    @mock.patch('gevent.os.get_hub')
+    @mock.patch('gevent.os._write')
+    def test_event_closed_on_write(self, mock_write, mock_get_hub):
+        self._test_event_closed_on_normal_io(os.nb_write, b'buf',
+                                             mock_write, mock_get_hub,
+                                             2)
+
+    @mock.patch('gevent.os.get_hub')
+    @mock.patch('gevent.os._write')
+    def test_event_closed_on_write_error(self, mock_write, mock_get_hub):
+        self._test_event_closed_on_io_error(os.nb_write, b'buf',
+                                            mock_write, mock_get_hub,
+                                            2)
+
+    @mock.patch('gevent.os.get_hub')
+    @mock.patch('gevent.os._read')
+    def test_event_closed_on_read(self, mock_read, mock_get_hub):
+        self._test_event_closed_on_normal_io(os.nb_read, b'buf',
+                                             mock_read, mock_get_hub,
+                                             1)
+
+    @mock.patch('gevent.os.get_hub')
+    @mock.patch('gevent.os._read')
+    def test_event_closed_on_read_error(self, mock_read, mock_get_hub):
+        self._test_event_closed_on_io_error(os.nb_read, b'buf',
+                                            mock_read, mock_get_hub,
+                                            1)
+
+
+@greentest.skipUnless(hasattr(os, 'fork_and_watch'),
+                      "Only on POSIX")
+class TestForkAndWatch(greentest.TestCase):
+
+    __timeout__ = greentest.LARGE_TIMEOUT
+
+    def test_waitpid_all(self):
+        # Cover this specific case.
+        pid = os.fork_and_watch()
+        if pid:
+            os.waitpid(-1, 0)
+            # Can't assert on what the pid actually was,
+            # our testrunner may have spawned multiple children.
+            os._reap_children(0) # make the leakchecker happy
+        else: # pragma: no cover
+            gevent.sleep(2)
+            os._exit(0)
+
+    def test_waitpid_wrong_neg(self):
+        self.assertRaises(OSError, os.waitpid, -2, 0)
+
+    def test_waitpid_wrong_pos(self):
+        self.assertRaises(OSError, os.waitpid, 1, 0)
 
-        def test_waitpid_wrong_pos(self):
-            self.assertRaises(OSError, os.waitpid, 1, 0)
 
 if __name__ == '__main__':
-    main()
+    greentest.main()