From dbb78a0265c23cb05cbe5df2b79eab22b53501fc Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Thu, 5 Mar 2020 22:10:14 -0500 Subject: [PATCH 1/4] TST: add the request fixture to check_figures_equal If you stacked `pytest.mark.parametrize` with check_figures_equal every set of parameters would write to the same file. This makes post-hoc debugging hard and causes intermittent CI failures. --- lib/matplotlib/testing/decorators.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/testing/decorators.py b/lib/matplotlib/testing/decorators.py index 65d030ed9afe..1ed8be497c3e 100644 --- a/lib/matplotlib/testing/decorators.py +++ b/lib/matplotlib/testing/decorators.py @@ -388,15 +388,14 @@ def decorator(func): _, result_dir = _image_directories(func) @pytest.mark.parametrize("ext", extensions) - def wrapper(*args, ext, **kwargs): + def wrapper(*args, ext, request, **kwargs): + fn = request.node.name try: fig_test = plt.figure("test") fig_ref = plt.figure("reference") func(*args, fig_test=fig_test, fig_ref=fig_ref, **kwargs) - test_image_path = result_dir / (func.__name__ + "." + ext) - ref_image_path = result_dir / ( - func.__name__ + "-expected." + ext - ) + test_image_path = result_dir / (fn + "." + ext) + ref_image_path = result_dir / (fn + "-expected." + ext) fig_test.savefig(test_image_path) fig_ref.savefig(ref_image_path) _raise_on_image_difference( @@ -411,7 +410,9 @@ def wrapper(*args, ext, **kwargs): parameters=([param for param in sig.parameters.values() if param.name not in {"fig_test", "fig_ref"}] - + [inspect.Parameter("ext", POSITIONAL_OR_KEYWORD)]) + + [inspect.Parameter("ext", POSITIONAL_OR_KEYWORD), + inspect.Parameter("request", POSITIONAL_OR_KEYWORD), + ]) ) wrapper.__signature__ = new_sig From 3b57fba1ff5a033e9cfec759efca378331218ed7 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Fri, 6 Mar 2020 09:26:48 -0500 Subject: [PATCH 2/4] MNT: correctly label the inject kwargs as kwonly They are preceded by a `*args` in the signature of wrapper. --- lib/matplotlib/testing/decorators.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/matplotlib/testing/decorators.py b/lib/matplotlib/testing/decorators.py index 1ed8be497c3e..a6b627678509 100644 --- a/lib/matplotlib/testing/decorators.py +++ b/lib/matplotlib/testing/decorators.py @@ -381,7 +381,7 @@ def test_plot(fig_test, fig_ref): fig_test.subplots().plot([1, 3, 5]) fig_ref.subplots().plot([0, 1, 2], [1, 3, 5]) """ - POSITIONAL_OR_KEYWORD = inspect.Parameter.POSITIONAL_OR_KEYWORD + KEYWORD_ONLY = inspect.Parameter.KEYWORD_ONLY def decorator(func): import pytest @@ -410,9 +410,10 @@ def wrapper(*args, ext, request, **kwargs): parameters=([param for param in sig.parameters.values() if param.name not in {"fig_test", "fig_ref"}] - + [inspect.Parameter("ext", POSITIONAL_OR_KEYWORD), - inspect.Parameter("request", POSITIONAL_OR_KEYWORD), - ]) + + [ + inspect.Parameter("ext", KEYWORD_ONLY), + inspect.Parameter("request", KEYWORD_ONLY), + ]) ) wrapper.__signature__ = new_sig From a66d8d8ea14bcdca82670f4d0945fb03042d0a30 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Sat, 14 Mar 2020 22:15:21 -0400 Subject: [PATCH 3/4] MNT: filter filename to not fail non-allowed names This is probably to stringent but this is better than blindly using the test name from pytest. --- lib/matplotlib/testing/decorators.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/matplotlib/testing/decorators.py b/lib/matplotlib/testing/decorators.py index a6b627678509..a2db53215a07 100644 --- a/lib/matplotlib/testing/decorators.py +++ b/lib/matplotlib/testing/decorators.py @@ -8,6 +8,7 @@ import sys import unittest import warnings +import string import matplotlib as mpl import matplotlib.style @@ -381,6 +382,7 @@ def test_plot(fig_test, fig_ref): fig_test.subplots().plot([1, 3, 5]) fig_ref.subplots().plot([0, 1, 2], [1, 3, 5]) """ + ALLOWED_CHARS = set(string.digits + string.ascii_letters + '_-[]()') KEYWORD_ONLY = inspect.Parameter.KEYWORD_ONLY def decorator(func): import pytest @@ -389,13 +391,14 @@ def decorator(func): @pytest.mark.parametrize("ext", extensions) def wrapper(*args, ext, request, **kwargs): - fn = request.node.name + file_name = "".join(c for c in request.node.name + if c in ALLOWED_CHARS) try: fig_test = plt.figure("test") fig_ref = plt.figure("reference") func(*args, fig_test=fig_test, fig_ref=fig_ref, **kwargs) - test_image_path = result_dir / (fn + "." + ext) - ref_image_path = result_dir / (fn + "-expected." + ext) + test_image_path = result_dir / (file_name + "." + ext) + ref_image_path = result_dir / (file_name + "-expected." + ext) fig_test.savefig(test_image_path) fig_ref.savefig(ref_image_path) _raise_on_image_difference( From 3788e3cd31fae3b74586c8449075847c29fb9a03 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Sat, 14 Mar 2020 22:17:51 -0400 Subject: [PATCH 4/4] MNT: remove unused import --- lib/matplotlib/testing/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/matplotlib/testing/decorators.py b/lib/matplotlib/testing/decorators.py index a2db53215a07..1d1aa519aad6 100644 --- a/lib/matplotlib/testing/decorators.py +++ b/lib/matplotlib/testing/decorators.py @@ -18,7 +18,7 @@ from matplotlib import ft2font from matplotlib import pyplot as plt from matplotlib import ticker -from . import is_called_from_pytest + from .compare import comparable_formats, compare_images, make_test_filename from .exceptions import ImageComparisonFailure