From 886157f157dc95690b459708311f8c609b0572fc Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Mon, 23 Aug 2021 22:42:21 -0400 Subject: [PATCH 1/6] TST: add pyqt on appveyor --- .appveyor.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.appveyor.yml b/.appveyor.yml index 0e8e0a553fd4..c637dae4d869 100644 --- 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% From 2974f3ca9c86fcae57cc477f2e725e8147d85d0e Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 25 Aug 2021 20:41:07 -0400 Subject: [PATCH 2/6] Move sigint tests into subprocesses. This prevents them accidentally breaking the test runner itself, depending on platform. --- lib/matplotlib/tests/test_backend_qt.py | 162 +++++++++++++++++------- 1 file changed, 117 insertions(+), 45 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_qt.py b/lib/matplotlib/tests/test_backend_qt.py index b22772c35f78..fa941d072c89 100644 --- 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,134 @@ 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 InterruptiblePopen(subprocess.Popen): + """ + A Popen that passes flags that allow triggering KeyboardInterrupt. + """ + + def __init__(self, *args, **kwargs): + if sys.platform == 'win32': + kwargs['creationflags'] = subprocess.CREATE_NEW_PROCESS_GROUP + 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 interrupt(self): + """Interrupt process in a platform-specific way.""" + if sys.platform == 'win32': + self.send_signal(signal.CTRL_C_EVENT) + else: + self.send_signal(signal.SIGINT) + + +def _test_sigint_impl(backend, target_name, kwargs): + import sys + import matplotlib.pyplot as plt + plt.switch_backend(backend) + from matplotlib.backends.qt_compat import QtCore - qt_core.QTimer.singleShot(100, fire_signal) - with pytest.raises(KeyboardInterrupt): + target = getattr(plt, target_name) + + fig = plt.figure() + fig.canvas.mpl_connect('draw_event', + lambda *args: print('DRAW', flush=True)) + 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 = InterruptiblePopen( + [sys.executable, "-c", + inspect.getsource(_test_sigint_impl) + + f"\n_test_sigint_impl({backend!r}, {target!r}, {kwargs!r})"]) + try: + proc.wait_for('DRAW') + proc.interrupt() + 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 = InterruptiblePopen( + [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') + proc.interrupt() + 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') @@ -548,8 +622,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): From df090437c1dc8cce9e46305d0ea55ad31b1f83e9 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 25 Aug 2021 23:30:37 -0400 Subject: [PATCH 3/6] Ensure test_fig_sigint_override cleans up global state. --- lib/matplotlib/tests/test_backend_qt.py | 36 +++++++++++++------------ 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_qt.py b/lib/matplotlib/tests/test_backend_qt.py index fa941d072c89..b0a052c5c1fe 100644 --- a/lib/matplotlib/tests/test_backend_qt.py +++ b/lib/matplotlib/tests/test_backend_qt.py @@ -214,29 +214,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( From 9e84366c0debd78906f888dc2cce2248c4a41e4d Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 15 Sep 2021 16:00:13 -0400 Subject: [PATCH 4/6] FIX: try @vdrhtc to fix ctrl-c on windows --- lib/matplotlib/backends/qt_compat.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/backends/qt_compat.py b/lib/matplotlib/backends/qt_compat.py index 6fbe103ce03b..f0568d48b62d 100644 --- a/lib/matplotlib/backends/qt_compat.py +++ b/lib/matplotlib/backends/qt_compat.py @@ -228,8 +228,14 @@ def _maybe_allow_interrupt(qapp): rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read ) + 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 From 9d13e7d14f864dbb3b5ea35f74d02f649596ac0d Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Thu, 16 Sep 2021 18:58:16 -0400 Subject: [PATCH 5/6] TST: fix ctrl-c tests on windows --- lib/matplotlib/tests/test_backend_qt.py | 41 +++++++++++++++---------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_qt.py b/lib/matplotlib/tests/test_backend_qt.py index b0a052c5c1fe..7bdc01fe0223 100644 --- a/lib/matplotlib/tests/test_backend_qt.py +++ b/lib/matplotlib/tests/test_backend_qt.py @@ -54,14 +54,14 @@ def test_fig_close(): assert init_figs == Gcf.figs -class InterruptiblePopen(subprocess.Popen): +class WaitForStringPopen(subprocess.Popen): """ A Popen that passes flags that allow triggering KeyboardInterrupt. """ def __init__(self, *args, **kwargs): if sys.platform == 'win32': - kwargs['creationflags'] = subprocess.CREATE_NEW_PROCESS_GROUP + kwargs['creationflags'] = subprocess.CREATE_NEW_CONSOLE super().__init__( *args, **kwargs, # Force Agg so that each test can switch to its desired Qt backend. @@ -80,25 +80,35 @@ def wait_for(self, terminator): if buf.endswith(terminator): return - def interrupt(self): - """Interrupt process in a platform-specific way.""" - if sys.platform == 'win32': - self.send_signal(signal.CTRL_C_EVENT) - else: - self.send_signal(signal.SIGINT) - 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 - target = getattr(plt, target_name) + 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: print('DRAW', flush=True) + ) + fig.canvas.mpl_connect( + 'draw_event', + lambda *args: timer.start() + ) try: target(**kwargs) except KeyboardInterrupt: @@ -112,13 +122,12 @@ def _test_sigint_impl(backend, target_name, kwargs): ]) def test_sigint(target, kwargs): backend = plt.get_backend() - proc = InterruptiblePopen( + 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') - proc.interrupt() stdout, _ = proc.communicate(timeout=_test_timeout) except: proc.kill() @@ -164,7 +173,7 @@ def custom_signal_handler(signum, frame): ]) def test_other_signal_before_sigint(target, kwargs): backend = plt.get_backend() - proc = InterruptiblePopen( + proc = WaitForStringPopen( [sys.executable, "-c", inspect.getsource(_test_other_signal_before_sigint_impl) + "\n_test_other_signal_before_sigint_impl(" @@ -173,7 +182,7 @@ def test_other_signal_before_sigint(target, kwargs): proc.wait_for('DRAW') os.kill(proc.pid, signal.SIGUSR1) proc.wait_for('SIGUSR1') - proc.interrupt() + os.kill(proc.pid, signal.SIGINT) stdout, _ = proc.communicate(timeout=_test_timeout) except: proc.kill() From e13caa4805dc320a774e945122256a071f3be0b5 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Thu, 30 Sep 2021 15:39:45 -0400 Subject: [PATCH 6/6] DOC: add note about why reading the socket is complicated --- lib/matplotlib/backends/qt_compat.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/matplotlib/backends/qt_compat.py b/lib/matplotlib/backends/qt_compat.py index f0568d48b62d..9e320e341c4c 100644 --- a/lib/matplotlib/backends/qt_compat.py +++ b/lib/matplotlib/backends/qt_compat.py @@ -228,6 +228,12 @@ 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