From 6253d377fb42628a2ebe98c6fccdfa979b978143 Mon Sep 17 00:00:00 2001 From: Richard Sheridan Date: Fri, 14 Aug 2020 14:40:36 -0400 Subject: [PATCH 1/7] Migrate tk backend tests into subprocesses Try to isolate suspected interactions between tests --- lib/matplotlib/tests/test_backend_tk.py | 97 +++++++++++++++---- .../tests/test_backends_interactive.py | 29 ------ 2 files changed, 80 insertions(+), 46 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index 70eed6d75a81..d7dcd015c255 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -1,12 +1,12 @@ import os import subprocess import sys -import tkinter import numpy as np import pytest from matplotlib import pyplot as plt +_test_timeout = 10 # Empirically, 1s is not enough on Travis. @pytest.mark.backend('TkAgg', skip_on_importerror=True) @@ -36,25 +36,42 @@ def evil_blit(photoimage, aggimage, offsets, bboxptr): @pytest.mark.backend('TkAgg', skip_on_importerror=True) def test_figuremanager_preserves_host_mainloop(): - success = False - - def do_plot(): - plt.figure() - plt.plot([1, 2], [3, 5]) - plt.close() - root.after(0, legitimate_quit) + script = """ +import tkinter +import matplotlib.pyplot as plt +success = False - def legitimate_quit(): - root.quit() - nonlocal success - success = True +def do_plot(): + plt.figure() + plt.plot([1, 2], [3, 5]) + plt.close() + root.after(0, legitimate_quit) - root = tkinter.Tk() - root.after(0, do_plot) - root.mainloop() +def legitimate_quit(): + root.quit() + global success + success = True - assert success +root = tkinter.Tk() +root.after(0, do_plot) +root.mainloop() +if success: + print("success") +""" + try: + proc = subprocess.run( + [sys.executable, "-c", script,], + env={**os.environ, "SOURCE_DATE_EPOCH": "0"}, + timeout=_test_timeout, + stdout=subprocess.PIPE, + check=True, + universal_newlines=True, + ) + except subprocess.CalledProcessError: + pytest.fail("Subprocess failed to test intended behavior") + else: + assert proc.stdout.count("success") == 1 @pytest.mark.backend('TkAgg', skip_on_importerror=True) @pytest.mark.flaky(reruns=3) @@ -90,7 +107,7 @@ def target(): env={**os.environ, "MPLBACKEND": "TkAgg", "SOURCE_DATE_EPOCH": "0"}, - timeout=10, + timeout=_test_timeout, stdout=subprocess.PIPE, universal_newlines=True, check=True @@ -102,6 +119,52 @@ def target(): assert proc.stdout.count("success") == 1 +@pytest.mark.backend('TkAgg', skip_on_importerror=True) +@pytest.mark.flaky(reruns=3) +def test_never_update(): + script = """ +import tkinter +del tkinter.Misc.update +del tkinter.Misc.update_idletasks + +import matplotlib.pyplot as plt +fig = plt.figure() +plt.show(block=False) + +# regression test on FigureCanvasTkAgg +plt.draw() +# regression test on NavigationToolbar2Tk +fig.canvas.toolbar.configure_subplots() + +# check for update() or update_idletasks() in the event queue +# functionally equivalent to tkinter.Misc.update +# must pause >= 1 ms to process tcl idle events plus +# extra time to avoid flaky tests on slow systems +plt.pause(0.1) + +# regression test on FigureCanvasTk filter_destroy callback +plt.close(fig) +""" + try: + proc = subprocess.run( + [sys.executable, "-c", script,], + env={**os.environ, "SOURCE_DATE_EPOCH": "0"}, + timeout=_test_timeout, + capture_output=True, + check=True, + universal_newlines=True, + ) + except subprocess.CalledProcessError: + pytest.fail("Subprocess failed to test intended behavior") + else: + # test framework doesn't see tkinter callback exceptions normally + # see tkinter.Misc.report_callback_exception + assert "Exception in Tkinter callback" not in proc.stderr + finally: + # make sure we can see other issues + print(proc.stderr, file=sys.stderr) + + @pytest.mark.backend('TkAgg', skip_on_importerror=True) def test_missing_back_button(): from matplotlib.backends.backend_tkagg import NavigationToolbar2Tk diff --git a/lib/matplotlib/tests/test_backends_interactive.py b/lib/matplotlib/tests/test_backends_interactive.py index 4494efa35437..000ff971bc97 100644 --- a/lib/matplotlib/tests/test_backends_interactive.py +++ b/lib/matplotlib/tests/test_backends_interactive.py @@ -201,35 +201,6 @@ def test_webagg(): assert proc.wait(timeout=_test_timeout) == 0 -@pytest.mark.backend('TkAgg', skip_on_importerror=True) -def test_never_update(monkeypatch, capsys): - import tkinter - monkeypatch.delattr(tkinter.Misc, 'update') - monkeypatch.delattr(tkinter.Misc, 'update_idletasks') - - import matplotlib.pyplot as plt - fig = plt.figure() - plt.show(block=False) - - # regression test on FigureCanvasTkAgg - plt.draw() - # regression test on NavigationToolbar2Tk - fig.canvas.toolbar.configure_subplots() - - # check for update() or update_idletasks() in the event queue - # functionally equivalent to tkinter.Misc.update - # must pause >= 1 ms to process tcl idle events plus - # extra time to avoid flaky tests on slow systems - plt.pause(0.1) - - # regression test on FigureCanvasTk filter_destroy callback - plt.close(fig) - - # test framework doesn't see tkinter callback exceptions normally - # see tkinter.Misc.report_callback_exception - assert "Exception in Tkinter callback" not in capsys.readouterr().err - - @pytest.mark.skipif(sys.platform != "linux", reason="this a linux-only test") @pytest.mark.backend('Qt5Agg', skip_on_importerror=True) def test_lazy_linux_headless(): From 96caee272fc19637d763dc4ac1e78bd3c5e90565 Mon Sep 17 00:00:00 2001 From: Richard Sheridan Date: Fri, 14 Aug 2020 18:31:13 -0400 Subject: [PATCH 2/7] Better behavior in case of timeout --- lib/matplotlib/tests/test_backend_tk.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index d7dcd015c255..3de48c5283c1 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -73,6 +73,7 @@ def legitimate_quit(): else: assert proc.stdout.count("success") == 1 + @pytest.mark.backend('TkAgg', skip_on_importerror=True) @pytest.mark.flaky(reruns=3) def test_figuremanager_cleans_own_mainloop(): @@ -151,18 +152,18 @@ def test_never_update(): env={**os.environ, "SOURCE_DATE_EPOCH": "0"}, timeout=_test_timeout, capture_output=True, - check=True, universal_newlines=True, ) - except subprocess.CalledProcessError: - pytest.fail("Subprocess failed to test intended behavior") + except subprocess.TimeoutExpired: + pytest.fail("Subprocess timed out") else: # test framework doesn't see tkinter callback exceptions normally # see tkinter.Misc.report_callback_exception assert "Exception in Tkinter callback" not in proc.stderr - finally: # make sure we can see other issues print(proc.stderr, file=sys.stderr) + if proc.returncode: + pytest.fail("Subprocess failed to test intended behavior") @pytest.mark.backend('TkAgg', skip_on_importerror=True) From 725f1b988492533fe4fc2eec27af87b5eae4ffd1 Mon Sep 17 00:00:00 2001 From: richardsheridan Date: Fri, 14 Aug 2020 21:19:25 -0400 Subject: [PATCH 3/7] Consistent environments for subprocesses H/T @QuLogic --- lib/matplotlib/tests/test_backend_tk.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index 3de48c5283c1..cf1363bd6bc9 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -62,7 +62,9 @@ def legitimate_quit(): try: proc = subprocess.run( [sys.executable, "-c", script,], - env={**os.environ, "SOURCE_DATE_EPOCH": "0"}, + env={**os.environ, + "MPLBACKEND": "TkAgg", + "SOURCE_DATE_EPOCH": "0"}, timeout=_test_timeout, stdout=subprocess.PIPE, check=True, @@ -149,7 +151,9 @@ def test_never_update(): try: proc = subprocess.run( [sys.executable, "-c", script,], - env={**os.environ, "SOURCE_DATE_EPOCH": "0"}, + env={**os.environ, + "MPLBACKEND": "TkAgg", + "SOURCE_DATE_EPOCH": "0"}, timeout=_test_timeout, capture_output=True, universal_newlines=True, From 9371cb42f7255ce5ae44b259b4b200b889d1fc39 Mon Sep 17 00:00:00 2001 From: Richard Sheridan Date: Sat, 15 Aug 2020 09:49:17 -0400 Subject: [PATCH 4/7] linting --- lib/matplotlib/tests/test_backend_tk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index cf1363bd6bc9..9d50c14daf49 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -61,7 +61,7 @@ def legitimate_quit(): """ try: proc = subprocess.run( - [sys.executable, "-c", script,], + [sys.executable, "-c", script], env={**os.environ, "MPLBACKEND": "TkAgg", "SOURCE_DATE_EPOCH": "0"}, @@ -150,7 +150,7 @@ def test_never_update(): """ try: proc = subprocess.run( - [sys.executable, "-c", script,], + [sys.executable, "-c", script], env={**os.environ, "MPLBACKEND": "TkAgg", "SOURCE_DATE_EPOCH": "0"}, From 0ed7a3057123d15d4f9192f4992c74fc069b73f4 Mon Sep 17 00:00:00 2001 From: Richard Sheridan Date: Sat, 15 Aug 2020 09:50:30 -0400 Subject: [PATCH 5/7] universal subprocesses add note to continue the concept going forward --- lib/matplotlib/tests/test_backend_tk.py | 109 +++++++++++++++++------- 1 file changed, 77 insertions(+), 32 deletions(-) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index 9d50c14daf49..558c61f74812 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -2,36 +2,61 @@ import subprocess import sys -import numpy as np import pytest -from matplotlib import pyplot as plt _test_timeout = 10 # Empirically, 1s is not enough on Travis. +# NOTE: TkAgg tests seem to have interactions between tests, +# So isolate each test in a subprocess. See GH#18261 + @pytest.mark.backend('TkAgg', skip_on_importerror=True) def test_blit(): - from matplotlib.backends import _tkagg - def evil_blit(photoimage, aggimage, offsets, bboxptr): - data = np.asarray(aggimage) - height, width = data.shape[:2] - dataptr = (height, width, data.ctypes.data) - _tkagg.blit( - photoimage.tk.interpaddr(), str(photoimage), dataptr, offsets, - bboxptr) - - fig, ax = plt.subplots() - for bad_boxes in ((-1, 2, 0, 2), - (2, 0, 0, 2), - (1, 6, 0, 2), - (0, 2, -1, 2), - (0, 2, 2, 0), - (0, 2, 1, 6)): - with pytest.raises(ValueError): - evil_blit(fig.canvas._tkphoto, - np.ones((4, 4, 4)), - (0, 1, 2, 3), - bad_boxes) + script = """ +import matplotlib.pyplot as plt +import numpy as np +from matplotlib.backends import _tkagg +def evil_blit(photoimage, aggimage, offsets, bboxptr): + data = np.asarray(aggimage) + height, width = data.shape[:2] + dataptr = (height, width, data.ctypes.data) + _tkagg.blit( + photoimage.tk.interpaddr(), str(photoimage), dataptr, offsets, + bboxptr) + +fig, ax = plt.subplots() +bad_boxes = ((-1, 2, 0, 2), + (2, 0, 0, 2), + (1, 6, 0, 2), + (0, 2, -1, 2), + (0, 2, 2, 0), + (0, 2, 1, 6)) +for bad_box in bad_boxes: + try: + evil_blit(fig.canvas._tkphoto, + np.ones((4, 4, 4)), + (0, 1, 2, 3), + bad_box) + except ValueError: + print("success") +""" + try: + proc = subprocess.run( + [sys.executable, "-c", script], + env={**os.environ, + "MPLBACKEND": "TkAgg", + "SOURCE_DATE_EPOCH": "0"}, + timeout=_test_timeout, + stdout=subprocess.PIPE, + check=True, + universal_newlines=True, + ) + except subprocess.CalledProcessError: + pytest.fail("Likely regression on out-of-bounds data access" + " in _tkagg.cpp") + else: + print(proc.stdout) + assert proc.stdout.count("success") == 6 # len(bad_boxes) @pytest.mark.backend('TkAgg', skip_on_importerror=True) @@ -172,12 +197,32 @@ def test_never_update(): @pytest.mark.backend('TkAgg', skip_on_importerror=True) def test_missing_back_button(): - from matplotlib.backends.backend_tkagg import NavigationToolbar2Tk - class Toolbar(NavigationToolbar2Tk): - # only display the buttons we need - toolitems = [t for t in NavigationToolbar2Tk.toolitems if - t[0] in ('Home', 'Pan', 'Zoom')] - - fig = plt.figure() - # this should not raise - Toolbar(fig.canvas, fig.canvas.manager.window) + script = """ +import matplotlib.pyplot as plt +from matplotlib.backends.backend_tkagg import NavigationToolbar2Tk +class Toolbar(NavigationToolbar2Tk): + # only display the buttons we need + toolitems = [t for t in NavigationToolbar2Tk.toolitems if + t[0] in ('Home', 'Pan', 'Zoom')] + +fig = plt.figure() +print("setup complete") +# this should not raise +Toolbar(fig.canvas, fig.canvas.manager.window) +print("success") +""" + try: + proc = subprocess.run( + [sys.executable, "-c", script], + env={**os.environ, + "MPLBACKEND": "TkAgg", + "SOURCE_DATE_EPOCH": "0"}, + timeout=_test_timeout, + stdout=subprocess.PIPE, + universal_newlines=True, + ) + except subprocess.TimeoutExpired: + pytest.fail("Subprocess timed out") + else: + assert proc.stdout.count("setup complete") == 1 + assert proc.stdout.count("success") == 1 From 030943d11e33b3b66d432abcb0b2b700e4773572 Mon Sep 17 00:00:00 2001 From: richardsheridan Date: Mon, 17 Aug 2020 19:21:37 -0400 Subject: [PATCH 6/7] Consistent subprocess error checks and add comments --- lib/matplotlib/tests/test_backend_tk.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index 558c61f74812..9d02c9260614 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -51,6 +51,8 @@ def evil_blit(photoimage, aggimage, offsets, bboxptr): check=True, universal_newlines=True, ) + except subprocess.TimeoutExpired: + pytest.fail("Subprocess timed out") except subprocess.CalledProcessError: pytest.fail("Likely regression on out-of-bounds data access" " in _tkagg.cpp") @@ -95,6 +97,8 @@ def legitimate_quit(): check=True, universal_newlines=True, ) + except subprocess.TimeoutExpired: + pytest.fail("Subprocess timed out") except subprocess.CalledProcessError: pytest.fail("Subprocess failed to test intended behavior") else: @@ -191,6 +195,7 @@ def test_never_update(): assert "Exception in Tkinter callback" not in proc.stderr # make sure we can see other issues print(proc.stderr, file=sys.stderr) + # Checking return code late so the Tkinter assertion happens first if proc.returncode: pytest.fail("Subprocess failed to test intended behavior") @@ -226,3 +231,7 @@ class Toolbar(NavigationToolbar2Tk): else: assert proc.stdout.count("setup complete") == 1 assert proc.stdout.count("success") == 1 + # Checking return code late so the stdout assertions happen first + if proc.returncode: + pytest.fail("Subprocess failed to test intended behavior") + From 513034cb0e66f617c045ba7539c232eb87d8caa9 Mon Sep 17 00:00:00 2001 From: richardsheridan Date: Mon, 17 Aug 2020 19:30:59 -0400 Subject: [PATCH 7/7] linting --- lib/matplotlib/tests/test_backend_tk.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index 9d02c9260614..126813e7cb87 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -234,4 +234,3 @@ class Toolbar(NavigationToolbar2Tk): # Checking return code late so the stdout assertions happen first if proc.returncode: pytest.fail("Subprocess failed to test intended behavior") -