TST: Make image_comparison more pytest-y.

Instead of a heavy do-it-all class, split ImageComparisonDecorator into a smaller class that does just the comparison stuff and one that does only nose. For pytest, use a wrapper function that's decorated only by pytest decorators, and don't try to modify the function signature. By using a separate fixture, we can indirectly return the parameterized arguments instead. This stops pytest from getting confused about what takes what argument. The biggest benefit is that test code is now run as the *test*, whereas previously, it was run as the *setup* causing it to have all sorts of semantic irregularities.
matplotlib · tacaswell · Apr 16, 2017 · Mar 26, 2017 · Mar 26, 2017 · Mar 26, 2017
commit abc944549a4ae0b8f985d222655af2d22eb4941f
diff --git a/lib/matplotli 8000 b/sphinxext/tests/conftest.py b/lib/matplotli 8000 b/sphinxext/tests/conftest.py
@@ -2,4 +2,5 @@
                         unicode_literals)
 
 from matplotlib.testing.conftest import (mpl_test_settings,
+                                         mpl_image_comparison_parameters,
                                          pytest_configure, pytest_unconfigure)
diff --git a/lib/matplotlib/testing/conftest.py b/lib/matplotlib/testing/conftest.py
@@ -53,3 +53,19 @@ def mpl_test_settings(request):
             plt.switch_backend(prev_backend)
         _do_cleanup(original_units_registry,
                     original_settings)
+
+
+@pytest.fixture
+def mpl_image_comparison_parameters(request, extension):
+    # This fixture is applied automatically by the image_comparison decorator.
+    #
+    # The sole purpose of this fixture is to provide an indirect method of
+    # obtaining parameters *without* modifying the decorated function
+    # signature. In this way, the function signature can stay the same and
+    # pytest won't get confused.
+    # We annotate the decorated function with any parameters captured by this
+    # fixture so that they can be used by the wrapper in image_comparison.
+    func = request.function
+    func.__wrapped__.parameters = (extension, )
+    yield
+    delattr(func.__wrapped__, 'parameters')
diff --git a/lib/matplotlib/testing/decorators.py b/lib/matplotlib/testing/decorators.py
@@ -233,42 +233,18 @@ def _mark_xfail_if_format_is_uncomparable(extension):
         return extension
 
 
-class ImageComparisonDecorator(CleanupTest):
-    def __init__(self, baseline_images, extensions, tol,
-                 freetype_version, remove_text, savefig_kwargs, style):
+class _ImageComparisonBase(object):
+    def __init__(self, tol, remove_text, savefig_kwargs):
         self.func = self.baseline_dir = self.result_dir = None
-        self.baseline_images = baseline_images
-        self.extensions = extensions
         self.tol = tol
-        self.freetype_version = freetype_version
         self.remove_text = remove_text
         self.savefig_kwargs = savefig_kwargs
-        self.style = style
 
     def delayed_init(self, func):
         assert self.func is None, "it looks like same decorator used twice"
         self.func = func
         self.baseline_dir, self.result_dir = _image_directories(func)
 
-    def setup(self):
-        func = self.func
-        plt.close('all')
-        self.setup_class()
-        try:
-            matplotlib.style.use(self.style)
-            matplotlib.testing.set_font_settings_for_testing()
-            func()
-            assert len(plt.get_fignums()) == len(self.baseline_images), (
-                "Test generated {} images but there are {} baseline images"
-                .format(len(plt.get_fignums()), len(self.baseline_images)))
-        except:
            # Restore original settings before raising errors during the update.
-            self.teardown_class()
-            raise
-
-    def teardown(self):
-        self.teardown_class()
-
     def copy_baseline(self, baseline, extension):
         baseline_path = os.path.join(self.baseline_dir, baseline)
         orig_expected_fname = baseline_path + '.' + extension
@@ -304,6 +280,35 @@ def compare(self, idx, baseline, extension):
         expected_fname = self.copy_baseline(baseline, extension)
         _raise_on_image_difference(expected_fname, actual_fname, self.tol)
 
+
+class ImageComparisonDecorator(CleanupTest, _ImageComparisonBase):
+    def __init__(self, baseline_images, extensions, tol,
+                 freetype_version, remove_text, savefig_kwargs, style):
+        _ImageComparisonBase.__init__(self, tol, remove_text, savefig_kwargs)
+        self.baseline_images = baseline_images
+        self.extensions = extensions
+        self.freetype_version = freetype_version
+        self.style = style
+
+    def setup(self):
+        func = self.func
+        plt.close('all')
+        self.setup_class()
+        try:
+            matplotlib.style.use(self.style)
+            matplotlib.testing.set_font_settings_for_testing()
+            func()
+            assert len(plt.get_fignums()) == len(self.baseline_images), (
+                "Test generated {} images but there are {} baseline images"
+                .format(len(plt.get_fignums()), len(self.baseline_images)))
+        except:
+            # Restore original settings before raising errors.
+            self.teardown_class()
+            raise
+
+    def teardown(self):
+        self.teardown_class()
+
     def nose_runner(self):
         func = self.compare
         func = _checked_on_freetype_version(self.freetype_version)(func)
@@ -313,52 +318,59 @@ def nose_runner(self):
             for extension in self.extensions:
                 yield funcs[extension], idx, baseline, extension
 
-    def pytest_runner(self):
-        from pytest import mark
+    def __call__(self, func):
+        self.delayed_init(func)
+        import nose.tools
 
-        extensions = map(_mark_xfail_if_format_is_uncomparable,
-                         self.extensions)
+        @nose.tools.with_setup(self.setup, self.teardown)
+        def runner_wrapper():
+            try:
+                for case in self.nose_runner():
+                    yield case
+            except GeneratorExit:
+                # nose bug...
+                self.teardown()
 
-        if len(set(self.baseline_images)) == len(self.baseline_images):
-            @mark.parametrize("extension", extensions)
-            @mark.parametrize("idx,baseline", enumerate(self.baseline_images))
-            @_checked_on_freetype_version(self.freetype_version)
-            def wrapper(idx, baseline, extension):
-                __tracebackhide__ = True
-                self.compare(idx, baseline, extension)
-        else:
-            # Some baseline images are repeated, so run this in serial.
-            @mark.parametrize("extension", extensions)
-            @_checked_on_freetype_version(self.freetype_version)
-            def wrapper(extension):
-                __tracebackhide__ = True
-                for idx, baseline in enumerate(self.baseline_images):
-                    self.compare(idx, baseline, extension)
+        return _copy_metadata(func, runner_wrapper)
 
 
-        # sadly we cannot use fixture here because of visibility problems
-        # and for for obvious reason avoid `_nose.tools.with_setup`
-        wrapper.setup, wrapper.teardown = self.setup, self.teardown
+def _pytest_image_comparison(baseline_images, extensions, tol,
+                             freetype_version, remove_text, savefig_kwargs,
+                             style):
+    import pytest
 
-        return wrapper
+    extensions = map(_mark_xfail_if_format_is_uncomparable, extensions)
 
-    def __call__(self, func):
-        self.delayed_init(func)
-        if is_called_from_pytest():
-            return _copy_metadata(func, self.pytest_runner())
-        else:
-            import nose.tools
+    def decorator(func):
+        @pytest.mark.usefixtures('mpl_image_comparison_parameters')
+        @pytest.mark.paramet
6D40
rize('extension', extensions)
+        @pytest.mark.style(style)
+        @_checked_on_freetype_version(freetype_version)
+        @functools.wraps(func)
+        def wrapper(*args, **kwargs):
+            __tracebackhide__ = True
+            img = _ImageComparisonBase(tol=tol, remove_text=remove_text,
+                                       savefig_kwargs=savefig_kwargs)
+            img.delayed_init(func)
+            matplotlib.testing.set_font_settings_for_testing()
+            func(*args, **kwargs)
 
-            @nose.tools.with_setup(self.setup, self.teardown)
-            def runner_wrapper():
-                try:
-                    for case in self.nose_runner():
-                        yield case
-                except GeneratorExit:
-                    # nose bug...
-                    self.teardown()
+            # This is hacked on via the mpl_image_comparison_parameters fixture
+            # so that we don't need to modify the function's real signature for
+            # any parametrization. Modifying the signature is very very tricky
+            # and likely to confuse pytest.
+            extension, = func.parameters
+
+            assert len(plt.get_fignums()) == len(baseline_images), (
+                "Test generated {} images but there are {} baseline images"
+                .format(len(plt.get_fignums()), len(baseline_images)))
+            for idx, baseline in enumerate(baseline_images):
+                img.compare(idx, baseline, extension)
+
+        wrapper.__wrapped__ = func  # For Python 2.7.
+        return _copy_metadata(func, wrapper)
 
-            return _copy_metadata(func, runner_wrapper)
+    return decorator
 
 
 def image_comparison(baseline_images=None, extensions=None, tol=0,
@@ -414,10 +426,16 @@ def image_comparison(baseline_images=None, extensions=None, tol=0,
         #default no kwargs to savefig
         savefig_kwarg = dict()
 
-    return ImageComparisonDecorator(
-        baseline_images=baseline_images, extensions=extensions, tol=tol,
-        freetype_version=freetype_version, remove_text=remove_text,
-        savefig_kwargs=savefig_kwarg, style=style)
+    if is_called_from_pytest():
+        return _pytest_image_comparison(
+            baseline_images=baseline_images, extensions=extensions, tol=tol,
+            freetype_version=freetype_version, remove_text=remove_text,
+            savefig_kwargs=savefig_kwarg, style=style)
+    else:
+        return ImageComparisonDecorator(
+            baseline_images=baseline_images, extensions=extensions, tol=tol,
+            freetype_version=freetype_version, remove_text=remove_text,
+            savefig_kwargs=savefig_kwarg, style=style)
 
 
 def _image_directories(func):

diff --git a/lib/matplotlib/tests/conftest.py b/lib/matplotlib/tests/conftest.py
@@ -2,4 +2,5 @@
                         unicode_literals)
 
 from matplotlib.testing.conftest import (mpl_test_settings,
+                                         mpl_image_comparison_parameters,
                                          pytest_configure, pytest_unconfigure)
diff --git a/lib/mpl_toolkits/tests/conftest.py b/lib/mpl_toolkits/tests/conftest.py
@@ -2,4 +2,5 @@
                         unicode_literals)
 
 from matplotlib.testing.conftest import (mpl_test_settings,
+                                         mpl_image_comparison_parameters,
                                          pytest_configure, pytest_unconfigure)