Merge pull request #20907 from QuLogic/qt-interrupt-popen

jklymak · web-flow · commit 7bae7dce32bf · 2021-10-01T10:46:00.000+02:00
Move sigint tests into subprocesses
diff --git a/.appveyor.yml b/.appveyor.yml
@@ -60,6 +60,8 @@ install:
   # pull pywin32 from conda because on py38 there is something wrong with finding
   # the dlls when insalled from pip
   - conda install -c conda-forge pywin32
+  # install pyqt from conda-forge
+  - conda install -c conda-forge pyqt
   - echo %PYTHON_VERSION% %TARGET_ARCH%
   # Install dependencies from PyPI.
   - python -m pip install --upgrade -r requirements/testing/all.txt %EXTRAREQS% %PINNEDVERS%
diff --git a/lib/matplotlib/backends/qt_compat.py b/lib/matplotlib/backends/qt_compat.py
@@ -228,8 +228,20 @@ def _maybe_allow_interrupt(qapp):
             rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read
         )
 
+        # We do not actually care about this value other than running some
+        # Python code to ensure that the interpreter has a chance to handle the
+        # signal in Python land.  We also need to drain the socket because it
+        # will be written to as part of the wakeup!  There are some cases where
+        # this may fire too soon / more than once on Windows so we should be
+        # forgiving about reading an empty socket.
+        rsock.setblocking(False)
         # Clear the socket to re-arm the notifier.
-        sn.activated.connect(lambda *args: rsock.recv(1))
+        @sn.activated.connect
+        def _may_clear_sock(*args):
+            try:
+                rsock.recv(1)
+            except BlockingIOError:
+                pass
 
         def handle(*args):
             nonlocal handler_args
diff --git a/lib/matplotlib/tests/test_backend_qt.py b/lib/matplotlib/tests/test_backend_qt.py
@@ -24,6 +24,9 @@
     pytestmark = pytest.mark.skip('No usable Qt bindings')
 
 
+_test_timeout = 60  # A reasonably safe value for slower architectures.
+
+
 @pytest.fixture
 def qt_core(request):
     backend, = request.node.get_closest_marker('backend').args
@@ -33,19 +36,6 @@ def qt_core(request):
     return QtCore
 
 
-@pytest.fixture
-def platform_simulate_ctrl_c(request):
-    import signal
-    from functools import partial
-
-    if hasattr(signal, "CTRL_C_EVENT"):
-        win32api = pytest.importorskip('win32api')
-        return partial(win32api.GenerateConsoleCtrlEvent, 0, 0)
-    else:
-        # we're not on windows
-        return partial(os.kill, os.getpid(), signal.SIGINT)
-
-
 @pytest.mark.backend('QtAgg', skip_on_importerror=True)
 def test_fig_close():
 
@@ -64,50 +54,143 @@ def test_fig_close():
     assert init_figs == Gcf.figs
 
 
-@pytest.mark.backend('QtAgg', skip_on_importerror=True)
-@pytest.mark.parametrize("target, kwargs", [
-    (plt.show, {"block": True}),
-    (plt.pause, {"interval": 10})
-])
-def test_sigint(qt_core, platform_simulate_ctrl_c, target,
-                kwargs):
-    plt.figure()
-    def fire_signal():
-        platform_simulate_ctrl_c()
+class WaitForStringPopen(subprocess.Popen):
+    """
+    A Popen that passes flags that allow triggering KeyboardInterrupt.
+    """
 
-    qt_core.QTimer.singleShot(100, fire_signal)
-    with pytest.raises(KeyboardInterrupt):
+    def __init__(self, *args, **kwargs):
+        if sys.platform == 'win32':
+            kwargs['creationflags'] = subprocess.CREATE_NEW_CONSOLE
+        super().__init__(
+            *args, **kwargs,
+            # Force Agg so that each test can switch to its desired Qt backend.
+            env={**os.environ, "MPLBACKEND": "Agg", "SOURCE_DATE_EPOCH": "0"},
+            stdout=subprocess.PIPE, universal_newlines=True)
+
+    def wait_for(self, terminator):
+        """Read until the terminator is reached."""
+        buf = ''
+        while True:
+            c = self.stdout.read(1)
+            if not c:
+                raise RuntimeError(
+                    f'Subprocess died before emitting expected {terminator!r}')
+            buf += c
+            if buf.endswith(terminator):
+                return
+
+
+def _test_sigint_impl(backend, target_name, kwargs):
+    import sys
+    import matplotlib.pyplot as plt
+    import os
+    import threading
+
+    plt.switch_backend(backend)
+    from matplotlib.backends.qt_compat import QtCore
+
+    def interupter():
+        if sys.platform == 'win32':
+            import win32api
+            win32api.GenerateConsoleCtrlEvent(0, 0)
+        else:
+            import signal
+            os.kill(os.getpid(), signal.SIGINT)
+
+    target = getattr(plt, target_name)
+    timer = threading.Timer(1, interupter)
+    fig = plt.figure()
+    fig.canvas.mpl_connect(
+        'draw_event',
+        lambda *args: print('DRAW', flush=True)
+    )
+    fig.canvas.mpl_connect(
+        'draw_event',
+        lambda *args: timer.start()
+    )
+    try:
         target(**kwargs)
+    except KeyboardInterrupt:
+        print('SUCCESS', flush=True)
 
 
 @pytest.mark.backend('QtAgg', skip_on_importerror=True)
 @pytest.mark.parametrize("target, kwargs", [
-    (plt.show, {"block": True}),
-    (plt.pause, {"interval": 10})
+    ('show', {'block': True}),
+    ('pause', {'interval': 10})
 ])
-def test_other_signal_before_sigint(qt_core, platform_simulate_ctrl_c,
-                                    target, kwargs):
-    plt.figure()
+def test_sigint(target, kwargs):
+    backend = plt.get_backend()
+    proc = WaitForStringPopen(
+        [sys.executable, "-c",
+         inspect.getsource(_test_sigint_impl) +
+         f"\n_test_sigint_impl({backend!r}, {target!r}, {kwargs!r})"])
+    try:
+        proc.wait_for('DRAW')
+        stdout, _ = proc.communicate(timeout=_test_timeout)
+    except:
+        proc.kill()
+        stdout, _ = proc.communicate()
+        raise
+    print(stdout)
+    assert 'SUCCESS' in stdout
+
+
+def _test_other_signal_before_sigint_impl(backend, target_name, kwargs):
+    import signal
+    import sys
+    import matplotlib.pyplot as plt
+    plt.switch_backend(backend)
+    from matplotlib.backends.qt_compat import QtCore
 
-    sigcld_caught = False
-    def custom_sigpipe_handler(signum, frame):
-        nonlocal sigcld_caught
-        sigcld_caught = True
-    signal.signal(signal.SIGCHLD, custom_sigpipe_handler)
+    target = getattr(plt, target_name)
 
-    def fire_other_signal():
-        os.kill(os.getpid(), signal.SIGCHLD)
+    fig = plt.figure()
+    fig.canvas.mpl_connect('draw_event',
+                           lambda *args: print('DRAW', flush=True))
 
-    def fire_sigint():
-        platform_simulate_ctrl_c()
+    timer = fig.canvas.new_timer(interval=1)
+    timer.single_shot = True
+    timer.add_callback(print, 'SIGUSR1', flush=True)
 
-    qt_core.QTimer.singleShot(50, fire_other_signal)
-    qt_core.QTimer.singleShot(100, fire_sigint)
+    def custom_signal_handler(signum, frame):
+        timer.start()
+    signal.signal(signal.SIGUSR1, custom_signal_handler)
 
-    with pytest.raises(KeyboardInterrupt):
+    try:
         target(**kwargs)
+    except KeyboardInterrupt:
+        print('SUCCESS', flush=True)
 
-    assert sigcld_caught
+
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason='No other signal available to send on Windows')
+@pytest.mark.backend('QtAgg', skip_on_importerror=True)
+@pytest.mark.parametrize("target, kwargs", [
+    ('show', {'block': True}),
+    ('pause', {'interval': 10})
+])
+def test_other_signal_before_sigint(target, kwargs):
+    backend = plt.get_backend()
+    proc = WaitForStringPopen(
+        [sys.executable, "-c",
+         inspect.getsource(_test_other_signal_before_sigint_impl) +
+         "\n_test_other_signal_before_sigint_impl("
+            f"{backend!r}, {target!r}, {kwargs!r})"])
+    try:
+        proc.wait_for('DRAW')
+        os.kill(proc.pid, signal.SIGUSR1)
+        proc.wait_for('SIGUSR1')
+        os.kill(proc.pid, signal.SIGINT)
+        stdout, _ = proc.communicate(timeout=_test_timeout)
+    except:
+        proc.kill()
+        stdout, _ = proc.communicate()
+        raise
+    print(stdout)
+    assert 'SUCCESS' in stdout
+    plt.figure()
 
 
 @pytest.mark.backend('Qt5Agg')
@@ -140,29 +223,31 @@ def custom_handler(signum, frame):
 
     signal.signal(signal.SIGINT, custom_handler)
 
-    # mainloop() sets SIGINT, starts Qt event loop (which triggers timer and
-    # exits) and then mainloop() resets SIGINT
-    matplotlib.backends.backend_qt._BackendQT.mainloop()
+    try:
+        # mainloop() sets SIGINT, starts Qt event loop (which triggers timer
+        # and exits) and then mainloop() resets SIGINT
+        matplotlib.backends.backend_qt._BackendQT.mainloop()
 
-    # Assert: signal handler during loop execution is changed
-    # (can't test equality with func)
-    assert event_loop_handler != custom_handler
+        # Assert: signal handler during loop execution is changed
+        # (can't test equality with func)
+        assert event_loop_handler != custom_handler
 
-    # Assert: current signal handler is the same as the one we set before
-    assert signal.getsignal(signal.SIGINT) == custom_handler
+        # Assert: current signal handler is the same as the one we set before
+        assert signal.getsignal(signal.SIGINT) == custom_handler
 
-    # Repeat again to test that SIG_DFL and SIG_IGN will not be overridden
-    for custom_handler in (signal.SIG_DFL, signal.SIG_IGN):
-        qt_core.QTimer.singleShot(0, fire_signal_and_quit)
-        signal.signal(signal.SIGINT, custom_handler)
+        # Repeat again to test that SIG_DFL and SIG_IGN will not be overridden
+        for custom_handler in (signal.SIG_DFL, signal.SIG_IGN):
+            qt_core.QTimer.singleShot(0, fire_signal_and_quit)
+            signal.signal(signal.SIGINT, custom_handler)
 
-        _BackendQT5.mainloop()
+            _BackendQT5.mainloop()
 
-        assert event_loop_handler == custom_handler
-        assert signal.getsignal(signal.SIGINT) == custom_handler
+            assert event_loop_handler == custom_handler
+            assert signal.getsignal(signal.SIGINT) == custom_handler
 
-    # Reset SIGINT handler to what it was before the test
-    signal.signal(signal.SIGINT, original_handler)
+    finally:
+        # Reset SIGINT handler to what it was before the test
+        signal.signal(signal.SIGINT, original_handler)
 
 
 @pytest.mark.parametrize(
@@ -548,8 +633,6 @@ def _get_testable_qt_backends():
         envs.append(pytest.param(env, marks=marks, id=str(env)))
     return envs
 
-_test_timeout = 60  # A reasonably safe value for slower architectures.
-
 
 @pytest.mark.parametrize("env", _get_testable_qt_backends())
 def test_enums_available(env):
