review actions

Author: bjlittle

Diff for: pytest_mpl/kernels.py

@@ -118,6 +118,19 @@ def update_summary(self, summary):
         # The "name" class property *must* be defined in derived child class.
         summary["kernel"] = self.name
 
+    @property
+    def metadata(self):
+        """
+        The kernel metadata to be archived in a hash library with results.
+
+        Returns
+        -------
+        dict
+            The kernel metadata.
+
+        """
+        return dict(name=self.name)
+
 
 class KernelPHash(Kernel):
     """
@@ -138,15 +151,17 @@ def __init__(self, plugin):
         self.hamming_distance = None
         # Value may be overridden by py.test marker kwarg.
         arg = self._plugin.hamming_tolerance
-        self.hamming_tolerance = arg if arg is not None else DEFAULT_HAMMING_TOLERANCE
+        self.hamming_tolerance = (
+            int(arg) if arg is not None else DEFAULT_HAMMING_TOLERANCE
+        )
         # The hash-size (N) defines the resultant N**2 bits hash size.
         arg = self._plugin.hash_size
-        self.hash_size = arg if arg is not None else DEFAULT_HASH_SIZE
+        self.hash_size = int(arg) if arg is not None else DEFAULT_HASH_SIZE
         # The level of image detail (high freq) or structure (low freq)
         # represented in perceptual hash thru discrete cosine transform.
         arg = self._plugin.high_freq_factor
         self.high_freq_factor = (
-            arg if arg is not None else DEFAULT_HIGH_FREQUENCY_FACTOR
+            int(arg) if arg is not None else DEFAULT_HIGH_FREQUENCY_FACTOR
         )
         # py.test marker kwarg.
         self.option = "hamming_tolerance"
@@ -197,6 +212,13 @@ def update_summary(self, summary):
         summary["hamming_distance"] = self.hamming_distance
         summary["hamming_tolerance"] = self.hamming_tolerance
 
+    @property
+    def metadata(self):
+        result = super().metadata
+        result["hash_size"] = self.hash_size
+        result["high_freq_factor"] = self.high_freq_factor
+        return result
+
 
 class KernelSHA256(Kernel):
     """

Diff for: pytest_mpl/plugin.py

@@ -43,8 +43,7 @@
 
 import pytest
 
-from .kernels import (DEFAULT_HAMMING_TOLERANCE, DEFAULT_HASH_SIZE,
-                      DEFAULT_HIGH_FREQUENCY_FACTOR, KERNEL_SHA256, kernel_factory)
+from .kernels import KERNEL_SHA256, kernel_factory
 from .summary.html import generate_summary_basic_html, generate_summary_html
 
 #: The default matplotlib backend.
@@ -65,8 +64,8 @@
 #: The default matplotlib plot style.
 DEFAULT_STYLE = "classic"
 
-#: Metadata entry in the JSON hash library defining the source kernel of the hashes.
-META_HASH_LIBRARY_KERNEL = 'pytest-mpl-kernel'
+#: JSON metadata entry defining the source kernel of the hashes.
+META_HASH_KERNEL = 'pytest-mpl-kernel'
 
 #: Valid formats for generate summary.
 SUPPORTED_FORMATS = {'html', 'json', 'basic-html'}
@@ -326,20 +325,18 @@ def __init__(self,
 
         # Configure hashing kernel options.
         option = 'mpl-hash-size'
-        hash_size = int(config.getoption(f'--{option}') or
-                        config.getini(option) or DEFAULT_HASH_SIZE)
+        hash_size = (config.getoption(f'--{option}') or
+                     config.getini(option) or None)
         self.hash_size = hash_size
 
         option = 'mpl-hamming-tolerance'
-        hamming_tolerance = int(config.getoption(f'--{option}') or
-                                config.getini(option) or
-                                DEFAULT_HAMMING_TOLERANCE)
+        hamming_tolerance = (config.getoption(f'--{option}') or
+                             config.getini(option) or None)
         self.hamming_tolerance = hamming_tolerance
 
         option = 'mpl-high-freq-factor'
-        high_freq_factor = int(config.getoption(f'--{option}') or
-                               config.getini(option) or
-                               DEFAULT_HIGH_FREQUENCY_FACTOR)
+        high_freq_factor = (config.getoption(f'--{option}') or
+                            config.getini(option) or None)
         self.high_freq_factor = high_freq_factor
 
         # Configure the hashing kernel - must be done *after* kernel options.
@@ -351,12 +348,8 @@ def __init__(self,
                 emsg = f'Unrecognised hashing kernel {kernel!r} not supported.'
                 raise ValueError(emsg)
             kernel = requested
-            # Flag that the kernel has been user configured.
-            self.kernel_default = False
         else:
             kernel = DEFAULT_KERNEL
-            # Flag that the kernel has been configured by default.
-            self.kernel_default = True
         # Create the kernel.
         self.kernel = kernel_factory[kernel](self)
 
@@ -600,9 +593,63 @@ def compare_image_to_baseline(self, item, fig, result_dir, summary=None):
             summary['status_msg'] = error_message
             return error_message
 
-    def load_hash_library(self, library_path):
-        with open(str(library_path)) as fp:
-            return json.load(fp)
+    def load_hash_library(self, fname):
+        with open(str(fname)) as fi:
+            hash_library = json.load(fi)
+        kernel_metadata = hash_library.get(META_HASH_KERNEL)
+        if kernel_metadata is None:
+            msg = (f'Hash library {str(fname)!r} missing a '
+                   f'{META_HASH_KERNEL!r} entry. Assuming that a '
+                   f'{self.kernel.name!r} kernel generated the library.')
+            self.logger.info(msg)
+        else:
+            if "name" not in kernel_metadata:
+                emsg = (f"Missing kernel 'name' in the {META_HASH_KERNEL!r} entry, "
+                        f'for the hash library {str(fname)!r}.')
+                pytest.fail(emsg)
+            kernel_name = kernel_metadata["name"]
+            if kernel_name not in kernel_factory:
+                emsg = (f'Unrecognised hashing kernel {kernel_name!r} specified '
+                        f'in the hash library {str(fname)!r}.')
+                pytest.fail(emsg)
+            if kernel_name != self.kernel.name:
+                option = 'mpl-kernel'
+                if (self.config.getoption(f'--{option}') is None and
+                        len(self.config.getini(option)) == 0):
+                    # Override the default kernel with the kernel configured
+                    # within the hash library.
+                    self.kernel = kernel_factory[kernel_name](self)
+                else:
+                    emsg = (f'Hash library {str(fname)!r} kernel '
+                            f'{kernel_name!r} does not match configured runtime '
+                            f'kernel {self.kernel.name!r}.')
+                    pytest.fail(emsg)
+
+            def check_metadata(key):
+                if key not in kernel_metadata:
+                    emsg = (f'Missing kernel {key!r} in the '
+                            f'{META_HASH_KERNEL!r} entry, for the hash '
+                            f'library {str(fname)!r}.')
+                    pytest.fail(emsg)
+                value = kernel_metadata[key]
+                if value != getattr(self.kernel, key):
+                    option = f'mpl-{key.replace("_", "-")}'
+                    if (self.config.getoption(f'--{option}') is None and
+                            len(self.config.getini(option)) == 0):
+                        # Override the default kernel value with the
+                        # configured value within the hash library.
+                        setattr(self.kernel, key, value)
+                    else:
+                        emsg = (f"Hash library {str(fname)!r} '{key}={value}' "
+                                'does not match configured runtime kernel '
+                                f"'{key}={getattr(self.kernel, key)}'.")
+                        pytest.fail(emsg)
+
+            for key in self.kernel.metadata:
+                if key != "name":
+                    check_metadata(key)
+
+        return hash_library
 
     def compare_image_to_hash_library(self, item, fig, result_dir, summary=None):
         hash_comparison_pass = False
@@ -623,27 +670,6 @@ def compare_image_to_hash_library(self, item, fig, result_dir, summary=None):
             pytest.fail(f"Can't find hash library at path {str(hash_library_filename)!r}.")
 
         hash_library = self.load_hash_library(hash_library_filename)
-        kernel_name = hash_library.get(META_HASH_LIBRARY_KERNEL)
-        if kernel_name is None:
-            msg = (f'Hash library {str(hash_library_filename)!r} missing a '
-                   f'{META_HASH_LIBRARY_KERNEL!r} entry. Assuming that a '
-                   f'{self.kernel.name!r} kernel generated the library.')
-            self.logger.info(msg)
-        else:
-            if kernel_name not in kernel_factory:
-                emsg = (f'Unrecognised hashing kernel {kernel_name!r} specified '
-                        f'in the hash library {str(hash_library_filename)!r}.')
-                pytest.fail(emsg)
-            if kernel_name != self.kernel.name:
-                if self.kernel_default:
-                    # Override the default kernel with the kernel configured
-                    # within the hash library.
-                    self.kernel = kernel_factory[kernel_name](self)
-                else:
-                    emsg = (f'Hash library {str(hash_library_filename)!r} kernel '
-                            f'{kernel_name!r} does not match configured runtime '
-                            f'kernel {self.kernel.name!r}.')
-                    pytest.fail(emsg)
 
         hash_name = self.generate_test_name(item)
         baseline_hash = hash_library.get(hash_name, None)
@@ -838,7 +864,7 @@ def pytest_unconfigure(self, config):
             # It's safe to inject this metadata, as the key is an invalid Python
             # class/function/method name, therefore there's no possible
             # namespace conflict with user py.test marker decorated tokens.
-            self._generated_hash_library[META_HASH_LIBRARY_KERNEL] = self.kernel.name
+            self._generated_hash_library[META_HASH_KERNEL] = self.kernel.metadata
             with open(hash_library_path, "w") as fp:
                 json.dump(self._generated_hash_library, fp, indent=2)
             if self.results_always:  # Make accessible in results directory
@@ -849,7 +875,7 @@ def pytest_unconfigure(self, config):
             result_hashes = {k: v['result_hash'] for k, v in self._test_results.items()
                              if v['result_hash']}
             if len(result_hashes) > 0:  # At least one hash comparison test
-                result_hashes[META_HASH_LIBRARY_KERNEL] = self.kernel.name
+                result_hashes[META_HASH_KERNEL] = self.kernel.metadata
                 with open(result_hash_library, "w") as fp:
                     json.dump(result_hashes, fp, indent=2)
 

Diff for: tests/baseline/hashes/test_phash.json

@@ -1,4 +1,8 @@
 {
   "test.test_gen": "8fe8c01f7a45b01f6645ac1fe9572b2a807e2ae0d41a7ab085967aac87997eab",
-  "pytest-mpl-kernel": "phash"
+  "pytest-mpl-kernel": {
+    "name": "phash",
+    "hash_size": 16,
+    "high_freq_factor": 4
+  }
 }

Diff for: tests/baseline/hashes/test_phash_bad_kernel.json

@@ -0,0 +1,8 @@
+{
+  "test.test_gen": "8fe8c01f7a45b01f6645ac1fe9572b2a807e2ae0d41a7ab085967aac87997eab",
+  "pytest-mpl-kernel": {
+    "name": "wibble",
+    "hash_size": 16,
+    "high_freq_factor": 4
+  }
+}

Diff for: tests/baseline/hashes/test_phash_missing_hash_size.json

@@ -0,0 +1,7 @@
+{
+  "test.test_gen": "8fe8c01f7a45b01f6645ac1fe9572b2a807e2ae0d41a7ab085967aac87997eab",
+  "pytest-mpl-kernel": {
+    "name": "phash",
+    "high_freq_factor": 4
+  }
+}

Diff for: tests/baseline/hashes/test_phash_missing_high_freq_factor.json

@@ -0,0 +1,7 @@
+{
+  "test.test_gen": "8fe8c01f7a45b01f6645ac1fe9572b2a807e2ae0d41a7ab085967aac87997eab",
+  "pytest-mpl-kernel": {
+    "name": "phash",
+    "hash_size": 16
+  }
+}

Diff for: tests/baseline/hashes/test_phash_missing_name.json

@@ -0,0 +1,7 @@
+{
+  "test.test_gen": "8fe8c01f7a45b01f6645ac1fe9572b2a807e2ae0d41a7ab085967aac87997eab",
+  "pytest-mpl-kernel": {
+    "hash_size": 16,
+    "high_freq_factor": 4
+  }
+}

Diff for: tests/test_kernels.py

@@ -1,16 +1,11 @@
 from pathlib import Path
-from unittest.mock import sentinel
 
 import pytest
 
 from pytest_mpl.kernels import (DEFAULT_HAMMING_TOLERANCE, DEFAULT_HASH_SIZE,
-                                DEFAULT_HIGH_FREQUENCY_FACTOR, KERNEL_PHASH, KERNEL_SHA256, Kernel,
+                                DEFAULT_HIGH_FREQUENCY_FACTOR, Kernel,
                                 KernelPHash, KernelSHA256, kernel_factory)
 
-HASH_SIZE = sentinel.hash_size
-HAMMING_TOLERANCE = sentinel.hamming_tolerance
-HIGH_FREQUENCY_FACTOR = sentinel.high_freq_factor
-
 #: baseline hash (32-bit)
 HASH_BASE_32 = "01234567"
 
@@ -69,15 +64,16 @@ def test_phash_name():
 
 
 def test_phash_init__set():
+    hash_size, hamming_tolerance, high_freq_factor = -1, -2, -3
     plugin = DummyPlugin(
-        hash_size=HASH_SIZE,
-        hamming_tolerance=HAMMING_TOLERANCE,
-        high_freq_factor=HIGH_FREQUENCY_FACTOR,
+        hash_size=hash_size,
+        hamming_tolerance=hamming_tolerance,
+        high_freq_factor=high_freq_factor,
     )
     kernel = KernelPHash(plugin)
-    assert kernel.hash_size == HASH_SIZE
-    assert kernel.hamming_tolerance == HAMMING_TOLERANCE
-    assert kernel.high_freq_factor == HIGH_FREQUENCY_FACTOR
+    assert kernel.hash_size == hash_size
+    assert kernel.hamming_tolerance == hamming_tolerance
+    assert kernel.high_freq_factor == high_freq_factor
     assert kernel.equivalent is None
     assert kernel.hamming_distance is None
 
@@ -209,6 +205,20 @@ def test_phash_update_summary(summary, distance, tolerance, count):
     assert len(summary) == count
 
 
+@pytest.mark.parametrize(
+    "hash_size,hff",
+    [(DEFAULT_HASH_SIZE, DEFAULT_HIGH_FREQUENCY_FACTOR), (32, 8)],
+)
+def test_phash_metadata(hash_size, hff):
+    plugin = DummyPlugin(hash_size=hash_size, high_freq_factor=hff)
+    kernel = KernelPHash(plugin)
+    metadata = kernel.metadata
+    assert {"name", "hash_size", "high_freq_factor"} == set(metadata)
+    assert metadata["name"] == KernelPHash.name
+    assert metadata["hash_size"] == hash_size
+    assert metadata["high_freq_factor"] == hff
+
+
 #
 # KernelSHA256
 #
@@ -234,7 +244,7 @@ def test_sha256_generate_hash():
 
 def test_sha256_update_status():
     kernel = KernelSHA256(DummyPlugin())
-    message = sentinel.message
+    message = "nop"
     result = kernel.update_status(message)
     assert result is message
 
@@ -246,3 +256,10 @@ def test_sha256_update_summary():
     assert len(summary) == 1
     assert "kernel" in summary
     assert summary["kernel"] == KernelSHA256.name
+
+
+def test_sha256_metadata():
+    kernel = KernelSHA256(DummyPlugin())
+    metadata = kernel.metadata
+    assert {"name"} == set(metadata)
+    assert metadata["name"] == KernelSHA256.name