From 91d23c44b5dcbbe4c7180eb57a342a107dc2dbb6 Mon Sep 17 00:00:00 2001 From: Conor MacBride Date: Wed, 1 Jun 2022 20:17:01 +0100 Subject: [PATCH 1/5] Add failure test for running inside class --- tests/test_pytest_mpl.py | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/test_pytest_mpl.py b/tests/test_pytest_mpl.py index 5ab4d8a0..fec94cb0 100644 --- a/tests/test_pytest_mpl.py +++ b/tests/test_pytest_mpl.py @@ -486,3 +486,46 @@ def test_results_always(tmpdir): assert image and not image_exists assert image not in html assert json_res[json_image_key] is None + + +TEST_FAILING_CLASS = """ +import pytest +import matplotlib.pyplot as plt +class TestClass(object): + @pytest.mark.mpl_image_compare + def test_fails(self): + fig = plt.figure() + ax = fig.add_subplot(1, 1, 1) + ax.plot([1, 2, 3]) + return fig +""" + +TEST_FAILING_CLASS_SETUP_METHOD = """ +import pytest +import matplotlib.pyplot as plt +class TestClassWithSetup: + def setup_method(self, method): + self.x = [1, 2, 3] + @pytest.mark.mpl_image_compare + def test_fails(self): + fig = plt.figure() + ax = fig.add_subplot(1, 1, 1) + ax.plot(self.x) + return fig +""" + + +@pytest.mark.parametrize("code", [TEST_FAILING_CLASS, TEST_FAILING_CLASS_SETUP_METHOD]) +def test_class_fail(code, tmpdir): + + test_file = tmpdir.join('test.py').strpath + with open(test_file, 'w') as f: + f.write(code) + + # Assert fails if hash library missing + assert_pytest_fails_with(['--mpl', test_file, '--mpl-hash-library=/not/a/path'], + "Can't find hash library at path") + + # If we don't use --mpl option, the test should succeed + code = call_pytest([test_file]) + assert code == 0 From cdb37c9046395eb30b2ec91df062f59653b47369 Mon Sep 17 00:00:00 2001 From: Conor MacBride Date: Wed, 1 Jun 2022 20:39:52 +0100 Subject: [PATCH 2/5] Test different versions of pytest --- .github/workflows/test_and_publish.yml | 4 ++++ tox.ini | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/.github/workflows/test_and_publish.yml b/.github/workflows/test_and_publish.yml index 4888ba44..5ff29ec4 100644 --- a/.github/workflows/test_and_publish.yml +++ b/.github/workflows/test_and_publish.yml @@ -33,6 +33,10 @@ jobs: - linux: py38-test-mpl33 - linux: py39-test-mpl34 - linux: py310-test-mpl35 + # Test different versions of pytest + - linux: py310-test-mpl35-pytestdev + - linux: py310-test-mpl35-pytest62 + - linux: py310-test-mpl35-pytest54 coverage: 'codecov' publish: diff --git a/tox.ini b/tox.ini index 47bbf80d..0460defe 100644 --- a/tox.ini +++ b/tox.ini @@ -28,6 +28,13 @@ deps = mpl34: matplotlib==3.4.* mpl35: matplotlib==3.5.1 mpldev: git+https://github.com/matplotlib/matplotlib.git#egg=matplotlib + pytest54: pytest==5.4.* + pytest60: pytest==6.0.* + pytest61: pytest==6.1.* + pytest62: pytest==6.2.* + pytest70: pytest==7.0.* + pytest71: pytest==7.1.* + pytestdev: git+https://github.com/pytest-dev/pytest.git#egg=pytest extras = test commands = From 8aadc6fc38d4ece52806880521ecb59a49c49b30 Mon Sep 17 00:00:00 2001 From: Conor MacBride Date: Thu, 2 Jun 2022 16:46:18 +0100 Subject: [PATCH 3/5] Use `pytest_pyfunc_call` hook to intercept figure Instead modifying the test function itself by wrapping it in a function which runs the tests, use pytest hooks to intercept the generated figure and then run the tests. This should be a more robust approach that doesn't need as many special cases to be hardcoded. I have also refactored the get_marker and get_compare functions/methods to simplify these. --- pytest_mpl/plugin.py | 233 +++++++++++++++++++------------------------ 1 file changed, 103 insertions(+), 130 deletions(-) diff --git a/pytest_mpl/plugin.py b/pytest_mpl/plugin.py index fd8fe03c..3d838d3c 100644 --- a/pytest_mpl/plugin.py +++ b/pytest_mpl/plugin.py @@ -33,13 +33,11 @@ import json import shutil import hashlib -import inspect import logging import tempfile import warnings import contextlib from pathlib import Path -from functools import wraps from urllib.request import urlopen import pytest @@ -83,6 +81,14 @@ def pathify(path): return Path(path + ext) +def _pytest_pyfunc_call(obj, pyfuncitem): + testfunction = pyfuncitem.obj + funcargs = pyfuncitem.funcargs + testargs = {arg: funcargs[arg] for arg in pyfuncitem._fixtureinfo.argnames} + obj.result = testfunction(**testargs) + return True + + def pytest_report_header(config, startdir): import matplotlib import matplotlib.ft2font @@ -211,13 +217,11 @@ def close_mpl_figure(fig): plt.close(fig) -def get_marker(item, marker_name): - if hasattr(item, 'get_closest_marker'): - return item.get_closest_marker(marker_name) - else: - # "item.keywords.get" was deprecated in pytest 3.6 - # See https://docs.pytest.org/en/latest/mark.html#updating-code - return item.keywords.get(marker_name) +def get_compare(item): + """ + Return the mpl_image_compare marker for the given item. + """ + return item.get_closest_marker("mpl_image_compare") def path_is_not_none(apath): @@ -278,12 +282,6 @@ def __init__(self, logging.basicConfig(level=level) self.logger = logging.getLogger('pytest-mpl') - def get_compare(self, item): - """ - Return the mpl_image_compare marker for the given item. - """ - return get_marker(item, 'mpl_image_compare') - def generate_filename(self, item): """ Given a pytest item, generate the figure filename. @@ -291,7 +289,7 @@ def generate_filename(self, item): if self.config.getini('mpl-use-full-test-name'): filename = self.generate_test_name(item) + '.png' else: - compare = self.get_compare(item) + compare = get_compare(item) # Find test name to use as plot name filename = compare.kwargs.get('filename', None) if filename is None: @@ -319,7 +317,7 @@ def baseline_directory_specified(self, item): """ Returns `True` if a non-default baseline directory is specified. """ - compare = self.get_compare(item) + compare = get_compare(item) item_baseline_dir = compare.kwargs.get('baseline_dir', None) return item_baseline_dir or self.baseline_dir or self.baseline_relative_dir @@ -330,7 +328,7 @@ def get_baseline_directory(self, item): Using the global and per-test configuration return the absolute baseline dir, if the baseline file is local else return base URL. """ - compare = self.get_compare(item) + compare = get_compare(item) baseline_dir = compare.kwargs.get('baseline_dir', None) if baseline_dir is None: if self.baseline_dir is None: @@ -394,7 +392,7 @@ def generate_baseline_image(self, item, fig): """ Generate reference figures. """ - compare = self.get_compare(item) + compare = get_compare(item) savefig_kwargs = compare.kwargs.get('savefig_kwargs', {}) if not os.path.exists(self.generate_dir): @@ -413,7 +411,7 @@ def generate_image_hash(self, item, fig): For a `matplotlib.figure.Figure`, returns the SHA256 hash as a hexadecimal string. """ - compare = self.get_compare(item) + compare = get_compare(item) savefig_kwargs = compare.kwargs.get('savefig_kwargs', {}) imgdata = io.BytesIO() @@ -436,7 +434,7 @@ def compare_image_to_baseline(self, item, fig, result_dir, summary=None): if summary is None: summary = {} - compare = self.get_compare(item) + compare = get_compare(item) tolerance = compare.kwargs.get('tolerance', 2) savefig_kwargs = compare.kwargs.get('savefig_kwargs', {}) @@ -510,7 +508,7 @@ def compare_image_to_hash_library(self, item, fig, result_dir, summary=None): if summary is None: summary = {} - compare = self.get_compare(item) + compare = get_compare(item) savefig_kwargs = compare.kwargs.get('savefig_kwargs', {}) if not self.results_hash_library_name: @@ -582,11 +580,13 @@ def compare_image_to_hash_library(self, item, fig, result_dir, summary=None): return return summary['status_msg'] - def pytest_runtest_setup(self, item): # noqa + @pytest.hookimpl(hookwrapper=True) + def pytest_runtest_call(self, item): # noqa - compare = self.get_compare(item) + compare = get_compare(item) if compare is None: + yield return import matplotlib.pyplot as plt @@ -600,95 +600,82 @@ def pytest_runtest_setup(self, item): # noqa remove_text = compare.kwargs.get('remove_text', False) backend = compare.kwargs.get('backend', 'agg') - original = item.function - - @wraps(item.function) - def item_function_wrapper(*args, **kwargs): - - with plt.style.context(style, after_reset=True), switch_backend(backend): - - # Run test and get figure object - if inspect.ismethod(original): # method - # In some cases, for example if setup_method is used, - # original appears to belong to an instance of the test - # class that is not the same as args[0], and args[0] is the - # one that has the correct attributes set up from setup_method - # so we ignore original.__self__ and use args[0] instead. - fig = original.__func__(*args, **kwargs) - else: # function - fig = original(*args, **kwargs) - - if remove_text: - remove_ticks_and_titles(fig) - - test_name = self.generate_test_name(item) - result_dir = self.make_test_results_dir(item) - - summary = { - 'status': None, - 'image_status': None, - 'hash_status': None, - 'status_msg': None, - 'baseline_image': None, - 'diff_image': None, - 'rms': None, - 'tolerance': None, - 'result_image': None, - 'baseline_hash': None, - 'result_hash': None, - } - - # What we do now depends on whether we are generating the - # reference images or simply running the test. - if self.generate_dir is not None: - summary['status'] = 'skipped' - summary['image_status'] = 'generated' - summary['status_msg'] = 'Skipped test, since generating image.' - generate_image = self.generate_baseline_image(item, fig) - if self.results_always: # Make baseline image available in HTML - result_image = (result_dir / "baseline.png").absolute() - shutil.copy(generate_image, result_image) - summary['baseline_image'] = \ - result_image.relative_to(self.results_dir).as_posix() - - if self.generate_hash_library is not None: - summary['hash_status'] = 'generated' - image_hash = self.generate_image_hash(item, fig) - self._generated_hash_library[test_name] = image_hash - summary['baseline_hash'] = image_hash - - # Only test figures if not generating images - if self.generate_dir is None: - # Compare to hash library - if self.hash_library or compare.kwargs.get('hash_library', None): - msg = self.compare_image_to_hash_library(item, fig, result_dir, summary=summary) - - # Compare against a baseline if specified - else: - msg = self.compare_image_to_baseline(item, fig, result_dir, summary=summary) - - close_mpl_figure(fig) - - if msg is None: - if not self.results_always: - shutil.rmtree(result_dir) - for image_type in ['baseline_image', 'diff_image', 'result_image']: - summary[image_type] = None # image no longer exists - else: - self._test_results[test_name] = summary - pytest.fail(msg, pytrace=False) + with plt.style.context(style, after_reset=True), switch_backend(backend): + + # Run test and get figure object + yield + fig = self.result + + if remove_text: + remove_ticks_and_titles(fig) + + test_name = self.generate_test_name(item) + result_dir = self.make_test_results_dir(item) + + summary = { + 'status': None, + 'image_status': None, + 'hash_status': None, + 'status_msg': None, + 'baseline_image': None, + 'diff_image': None, + 'rms': None, + 'tolerance': None, + 'result_image': None, + 'baseline_hash': None, + 'result_hash': None, + } + + # What we do now depends on whether we are generating the + # reference images or simply running the test. + if self.generate_dir is not None: + summary['status'] = 'skipped' + summary['image_status'] = 'generated' + summary['status_msg'] = 'Skipped test, since generating image.' + generate_image = self.generate_baseline_image(item, fig) + if self.results_always: # Make baseline image available in HTML + result_image = (result_dir / "baseline.png").absolute() + shutil.copy(generate_image, result_image) + summary['baseline_image'] = \ + result_image.relative_to(self.results_dir).as_posix() + + if self.generate_hash_library is not None: + summary['hash_status'] = 'generated' + image_hash = self.generate_image_hash(item, fig) + self._generated_hash_library[test_name] = image_hash + summary['baseline_hash'] = image_hash + + # Only test figures if not generating images + if self.generate_dir is None: + # Compare to hash library + if self.hash_library or compare.kwargs.get('hash_library', None): + msg = self.compare_image_to_hash_library(item, fig, result_dir, summary=summary) + + # Compare against a baseline if specified + else: + msg = self.compare_image_to_baseline(item, fig, result_dir, summary=summary) close_mpl_figure(fig) - self._test_results[test_name] = summary + if msg is None: + if not self.results_always: + shutil.rmtree(result_dir) + for image_type in ['baseline_image', 'diff_image', 'result_image']: + summary[image_type] = None # image no longer exists + else: + self._test_results[test_name] = summary + pytest.fail(msg, pytrace=False) + + close_mpl_figure(fig) - if summary['status'] == 'skipped': - pytest.skip(summary['status_msg']) + self._test_results[test_name] = summary - if item.cls is not None: - setattr(item.cls, item.function.__name__, item_function_wrapper) - else: - item.obj = item_function_wrapper + if summary['status'] == 'skipped': + pytest.skip(summary['status_msg']) + + @pytest.hookimpl(tryfirst=True) + def pytest_pyfunc_call(self, pyfuncitem): + return _pytest_pyfunc_call(self, pyfuncitem) def generate_summary_json(self): json_file = self.results_dir / 'results.json' @@ -742,26 +729,12 @@ class FigureCloser: def __init__(self, config): self.config = config - def pytest_runtest_setup(self, item): - - compare = get_marker(item, 'mpl_image_compare') - - if compare is None: - return - - original = item.function - - @wraps(item.function) - def item_function_wrapper(*args, **kwargs): - - if inspect.ismethod(original): # method - fig = original.__func__(*args, **kwargs) - else: # function - fig = original(*args, **kwargs) - - close_mpl_figure(fig) + @pytest.hookimpl(hookwrapper=True) + def pytest_runtest_call(self, item): + yield + if get_compare(item) is not None: + close_mpl_figure(self.result) - if item.cls is not None: - setattr(item.cls, item.function.__name__, item_function_wrapper) - else: - item.obj = item_function_wrapper + @pytest.hookimpl(tryfirst=True) + def pytest_pyfunc_call(self, pyfuncitem): + return _pytest_pyfunc_call(self, pyfuncitem) From 50251e565c06ec96b7e6c5393301511a94f0ac12 Mon Sep 17 00:00:00 2001 From: Conor MacBride Date: Thu, 2 Jun 2022 16:50:03 +0100 Subject: [PATCH 4/5] Run pytest 5 test on py38 --- .github/workflows/test_and_publish.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_and_publish.yml b/.github/workflows/test_and_publish.yml index 5ff29ec4..558f1b06 100644 --- a/.github/workflows/test_and_publish.yml +++ b/.github/workflows/test_and_publish.yml @@ -36,7 +36,7 @@ jobs: # Test different versions of pytest - linux: py310-test-mpl35-pytestdev - linux: py310-test-mpl35-pytest62 - - linux: py310-test-mpl35-pytest54 + - linux: py38-test-mpl35-pytest54 coverage: 'codecov' publish: From 19dac01ee8367089f0f0352fad9afaee098bbbb8 Mon Sep 17 00:00:00 2001 From: Conor MacBride Date: Thu, 2 Jun 2022 21:07:31 +0100 Subject: [PATCH 5/5] Include class name in generated test name --- pytest_mpl/plugin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pytest_mpl/plugin.py b/pytest_mpl/plugin.py index 3d838d3c..4613979c 100644 --- a/pytest_mpl/plugin.py +++ b/pytest_mpl/plugin.py @@ -302,7 +302,11 @@ def generate_test_name(self, item): """ Generate a unique name for the hash for this test. """ - return f"{item.module.__name__}.{item.name}" + if item.cls is not None: + name = f"{item.module.__name__}.{item.cls.__name__}.{item.name}" + else: + name = f"{item.module.__name__}.{item.name}" + return name def make_test_results_dir(self, item): """