Refactored image_comparison decorator

Kojoley · Kojoley · commit f8efcfccf208 · 2016-09-13T00:04:25.000+03:00
It is now function based, what removes the need of pytest workaround.
On pytest it uses `mark.parametrize` instead of a generator, what makes
collection phase lightning fast and significantly reduces number of generators
usages and corresponding deprecation warnings.
(generators are deprecated since pytest 3.0 and will be removed in 4.0)

Note: There is a bug in Nose related to `GeneratorExit` exception handling
in `setup` fixture of a function. It is workarounded in `image_comparison`
decorator, but you will see this line in such case in the log:
`RuntimeError: generator ignored GeneratorExit`
diff --git a/conftest.py b/conftest.py
@@ -10,7 +10,6 @@
 matplotlib.use('agg')
 
 from matplotlib import default_test_modules
-from matplotlib.testing.decorators import ImageComparisonTest
 
 
 IGNORED_TESTS = {
@@ -86,12 +85,6 @@ def pytest_ignore_collect(path, config):
 
 def pytest_pycollect_makeitem(collector, name, obj):
     if inspect.isclass(obj):
-        if issubclass(obj, ImageComparisonTest):
-            # Workaround `image_compare` decorator as it returns class
-            # instead of function and this confuses pytest because it crawls
-            # original names and sees 'test_*', but not 'Test*' in that case
-            return pytest.Class(name, parent=collector)
-
         if is_nose_class(obj) and not issubclass(obj, unittest.TestCase):
             # Workaround unittest-like setup/teardown names in pure classes
             setup = getattr(obj, 'setUp', None)
diff --git a/lib/matplotlib/testing/__init__.py b/lib/matplotlib/testing/__init__.py
@@ -62,7 +62,7 @@ def getrawcode(obj, trycall=True):
 
 def copy_metadata(src_func, tgt_func):
     """Replicates metadata of the function. Returns target function."""
-    tgt_func.__dict__ = src_func.__dict__
+    tgt_func.__dict__.update(src_func.__dict__)
     tgt_func.__doc__ = src_func.__doc__
     tgt_func.__module__ = src_func.__module__
     tgt_func.__name__ = src_func.__name__
diff --git a/lib/matplotlib/testing/decorators.py b/lib/matplotlib/testing/decorators.py
@@ -24,10 +24,9 @@
 from matplotlib import ticker
 from matplotlib import pyplot as plt
 from matplotlib import ft2font
-from matplotlib import rcParams
 from matplotlib.testing.compare import comparable_formats, compare_images, \
      make_test_filename
-from . import copy_metadata, is_called_from_pytest, skip, xfail
+from . import copy_metadata, is_called_from_pytest, xfail
 from .exceptions import ImageComparisonFailure
 
 
@@ -176,99 +175,175 @@ def check_freetype_version(ver):
     return found >= ver[0] and found <= ver[1]
 
 
-class ImageComparisonTest(CleanupTest):
-    @classmethod
-    def setup_class(cls):
-        CleanupTest.setup_class()
+def checked_on_freetype_version(required_freetype_version):
+    if check_freetype_version(required_freetype_version):
+        return lambda f: f
+
+    reason = ("Mismatched version of freetype. "
+              "Test requires '%s', you have '%s'" %
+              (required_freetype_version, ft2font.__freetype_version__))
+    return knownfailureif('indeterminate', msg=reason,
+                          known_exception_class=ImageComparisonFailure)
+
+
+def remove_ticks_and_titles(figure):
+    figure.suptitle("")
+    null_formatter = ticker.NullFormatter()
+    for ax in figure.get_axes():
+        ax.set_title("")
+        ax.xaxis.set_major_formatter(null_formatter)
+        ax.xaxis.set_minor_formatter(null_formatter)
+        ax.yaxis.set_major_formatter(null_formatter)
+        ax.yaxis.set_minor_formatter(null_formatter)
+        try:
+            ax.zaxis.set_major_formatter(null_formatter)
+            ax.zaxis.set_minor_formatter(null_formatter)
+        except AttributeError:
+            pass
+
+
+def raise_on_image_difference(expected, actual, tol):
+    __tracebackhide__ = True
+
+    err = compare_images(expected, actual, tol, in_decorator=True)
+
+    if not os.path.exists(expected):
+        raise ImageComparisonFailure('image does not exist: %s' % expected)
+
+    if err:
+        raise ImageComparisonFailure(
+            'images not close: %(actual)s vs. %(expected)s '
+            '(RMS %(rms).3f)' % err)
+
+
+def xfail_if_format_is_uncomparable(extension):
+    will_fail = extension not in comparable_formats()
+    if will_fail:
+        fail_msg = 'Cannot compare %s files on this system' % extension
+    else:
+        fail_msg = 'No failure expected'
+
+    return knownfailureif(will_fail, fail_msg,
+                          known_exception_class=ImageComparisonFailure)
+
+
+def mark_xfail_if_format_is_uncomparable(extension):
+    will_fail = extension not in comparable_formats()
+    if will_fail:
+        fail_msg = 'Cannot compare %s files on this system' % extension
+        import pytest
+        return pytest.mark.xfail(extension, reason=fail_msg, strict=False,
+                                 raises=ImageComparisonFailure)
+    else:
+        return extension
+
+
+class ImageComparisonDecorator(CleanupTest):
+    def __init__(self, baseline_images, extensions, tol,
+                 freetype_version, remove_text, savefig_kwargs, style):
+        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 setup(self):
+        func = self.func
+        self.setup_class()
         try:
-            matplotlib.style.use(cls._style)
+            matplotlib.style.use(self.style)
             matplotlib.testing.set_font_settings_for_testing()
-            cls._func()
+            func()
+            assert len(plt.get_fignums()) == len(self.baseline_images), (
+                'Figures and baseline_images count are not the same'
+                ' (`%s`)' % getattr(func, '__qualname__', func.__name__))
         except:
             # Restore original settings before raising errors during the update.
-            CleanupTest.teardown_class()
+            self.teardown_class()
             raise
 
-    @classmethod
-    def teardown_class(cls):
-        CleanupTest.teardown_class()
-
-    @staticmethod
-    def remove_text(figure):
-        figure.suptitle("")
-        for ax in figure.get_axes():
-            ax.set_title("")
-            ax.xaxis.set_major_formatter(ticker.NullFormatter())
-            ax.xaxis.set_minor_formatter(ticker.NullFormatter())
-            ax.yaxis.set_major_formatter(ticker.NullFormatter())
-            ax.yaxis.set_minor_formatter(ticker.NullFormatter())
-            try:
-                ax.zaxis.set_major_formatter(ticker.NullFormatter())
-                ax.zaxis.set_minor_formatter(ticker.NullFormatter())
-            except AttributeError:
-                pass
+    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
+        if extension == 'eps' and not os.path.exists(orig_expected_fname):
+            orig_expected_fname = baseline_path + '.pdf'
+        expected_fname = make_test_filename(os.path.join(
+            self.result_dir, os.path.basename(orig_expected_fname)), 'expected')
+        actual_fname = os.path.join(self.result_dir, baseline) + '.' + extension
+        if os.path.exists(orig_expected_fname):
+            shutil.copyfile(orig_expected_fname, expected_fname)
+        else:
+            xfail("Do not have baseline image {0} because this "
+                  "file does not exist: {1}".format(expected_fname,
+                                                    orig_expected_fname))
+        return expected_fname, actual_fname
+
+    def compare(self, idx, baseline, extension):
+        __tracebackhide__ = True
+        if self.baseline_dir is None:
+            self.baseline_dir, self.result_dir = _image_directories(self.func)
+        expected_fname, actual_fname = self.copy_baseline(baseline, extension)
+        fignum = plt.get_fignums()[idx]
+        fig = plt.figure(fignum)
+        if self.remove_text:
+            remove_ticks_and_titles(fig)
+        fig.savefig(actual_fname, **self.savefig_kwargs)
+        raise_on_image_difference(expected_fname, actual_fname, self.tol)
+
+    def nose_runner(self):
+        func = self.compare
+        func = skipif(self.style != 'classic',
+                      reason='temporarily disabled until 2.0 tag')(func)
+        func = checked_on_freetype_version(self.freetype_version)(func)
+        funcs = {extension: xfail_if_format_is_uncomparable(extension)(func)
+                 for extension in self.extensions}
+        for idx, baseline in enumerate(self.baseline_images):
+            for extension in self.extensions:
+                yield funcs[extension], idx, baseline, extension
+
+    def pytest_runner(self):
+        from pytest import mark
+
+        extensions = map(mark_xfail_if_format_is_uncomparable, self.extensions)
+
+        @mark.skipif(self.style != 'classic',
+                     reason='temporarily disabled until 2.0 tag')
+        @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)
+
+        # 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
+
+        return wrapper
+
+    def __call__(self, func):
+        self.func = func
+        if is_called_from_pytest():
+            return copy_metadata(func, self.pytest_runner())
+        else:
+            import nose.tools
 
-    def test(self):
-        baseline_dir, result_dir = _image_directories(self._func)
-        if self._style != 'classic':
-            skip('temporarily disabled until 2.0 tag')
-        for fignum, baseline in zip(plt.get_fignums(), self._baseline_images):
-            for extension in self._extensions:
-                will_fail = not extension in comparable_formats()
-                if will_fail:
-                    fail_msg = 'Cannot compare %s files on this system' % extension
-                else:
-                    fail_msg = 'No failure expected'
-
-                orig_expected_fname = os.path.join(baseline_dir, baseline) + '.' + extension
-                if extension == 'eps' and not os.path.exists(orig_expected_fname):
-                    orig_expected_fname = os.path.join(baseline_dir, baseline) + '.pdf'
-                expected_fname = make_test_filename(os.path.join(
-                    result_dir, os.path.basename(orig_expected_fname)), 'expected')
-                actual_fname = os.path.join(result_dir, baseline) + '.' + extension
-                if os.path.exists(orig_expected_fname):
-                    shutil.copyfile(orig_expected_fname, expected_fname)
-                else:
-                    will_fail = True
-                    fail_msg = (
-                        "Do not have baseline image {0} because this "
-                        "file does not exist: {1}".format(
-                            expected_fname,
-                            orig_expected_fname
-                        )
-                    )
-
-                @knownfailureif(
-                    will_fail, fail_msg,
-                    known_exception_class=ImageComparisonFailure)
-                def do_test(fignum, actual_fname, expected_fname):
-                    figure = plt.figure(fignum)
-
-                    if self._remove_text:
-                        self.remove_text(figure)
-
-                    figure.savefig(actual_fname, **self._savefig_kwarg)
-
-                    err = compare_images(expected_fname, actual_fname,
-                                         self._tol, in_decorator=True)
-
-                    try:
-                        if not os.path.exists(expected_fname):
-                            raise ImageComparisonFailure(
-                                'image does not exist: %s' % expected_fname)
-
-                        if err:
-                            raise ImageComparisonFailure(
-                                'images not close: %(actual)s vs. %(expected)s '
-                                '(RMS %(rms).3f)'%err)
-                    except ImageComparisonFailure:
-                        if not check_freetype_version(self._freetype_version):
-                            xfail(
-                                "Mismatched version of freetype.  Test requires '%s', you have '%s'" %
-                                (self._freetype_version, ft2font.__freetype_version__))
-                        raise
-
-                yield do_test, fignum, actual_fname, expected_fname
+            @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()
+
+            return copy_metadata(func, runner_wrapper)
 
 
 def image_comparison(baseline_images=None, extensions=None, tol=0,
@@ -327,35 +402,11 @@ def image_comparison(baseline_images=None, extensions=None, tol=0,
         #default no kwargs to savefig
         savefig_kwarg = dict()
 
-    def compare_images_decorator(func):
-        # We want to run the setup function (the actual test function
-        # that generates the figure objects) only once for each type
-        # of output file.  The only way to achieve this with nose
-        # appears to be to create a test class with "setup_class" and
-        # "teardown_class" methods.  Creating a class instance doesn't
-        # work, so we use type() to actually create a class and fill
-        # it with the appropriate methods.
-        name = func.__name__
-        # For nose 1.0, we need to rename the test function to
-        # something without the word "test", or it will be run as
-        # well, outside of the context of our image comparison test
-        # generator.
-        func = staticmethod(func)
-        func.__get__(1).__name__ = str('_private')
-        new_class = type(
-            name,
-            (ImageComparisonTest,),
-            {'_func': func,
-             '_baseline_images': baseline_images,
-             '_extensions': extensions,
-             '_tol': tol,
-             '_freetype_version': freetype_version,
-             '_remove_text': remove_text,
-             '_savefig_kwarg': savefig_kwarg,
-             '_style': style})
-
-        return new_class
-    return compare_images_decorator
+    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):
     """
