#62 Padding bug (#73)

gauge_web_app_steps/images.py

@@ -101,7 +101,7 @@ def adapt_and_compare_images(
         self.report.log_debug(f"actual channel_axis: {channel_axis}")
         self.report.log_debug(f"expected channel_axis: {self._channel_axis(img_expected)}")
         img_actual = self._rescale_image(img_actual, img_expected, channel_axis)
-        img_actual = self._pad_image(img_actual, img_expected)
+        img_actual, img_expected = self._pad_images(img_actual, img_expected)
         if img_actual is not img_actual_raw:
             self.report.log_debug("Overwriting actual image after rescaling and padding")
             skimg_io.imsave(actual_screenshot_full_path, img_actual)
@@ -131,7 +131,8 @@ def _align_alpha_channel_of_actual_image(self, img_expected: np.ndarray, img_act
             return rgba
         elif not expected_has_alpha and actual_has_alpha:
             self.report.log_debug("Removing alpha channel from actual image")
-            return skimg_color.rgba2rgb(img_actual)
+            img_rgb = skimg_color.rgba2rgb(img_actual)
+            return img_as_ubyte(img_rgb)
         else:
             return img_actual
 
@@ -162,7 +163,7 @@ def _rescale_image(
         )
         # skimage uses different internal representations for an image.
         # https://scikit-image.org/docs/dev/user_guide/data_types.html
-        # The rescale function return an image with a different data type, so we convert it back.
+        # The rescale function returns an image with a different data type, so we convert it back.
         return img_as_ubyte(img_rescaled)
 
     def _compute_rescale_ratio(
@@ -182,29 +183,39 @@ def _compute_rescale_ratio(
         width_ratio = 1.0 * reference_width / actual_width
         return height_ratio if height_ratio < width_ratio else width_ratio
 
-    def _pad_image(
+    def _pad_images(
             self,
             img,
             img_ref
-    ):
+    ) -> tuple[np.ndarray, np.ndarray]:
         pad_bottom = len(img_ref) - len(img)
         pad_right = len(img_ref[0]) - len(img[0])
         if pad_bottom == pad_right == 0:
-            return img
-        assert pad_bottom >= 0 and pad_right >= 0, \
-            "The actual picture overlaps the expected picture.\n" \
-            + "expected size: {}x{}, actual size: {}x{}".format(len(img_ref[0]), len(img_ref), len(img[0]), len(img))
-        self.report.log("screenshot needs padding to match expected image size. padding bottom: {}, padding right: {}" \
-            .format(pad_bottom, pad_right))
+            return img, img_ref
+        padded_ref_image = img_ref
+        padded_image = img
+        if pad_bottom < 0 or pad_right < 0:
+            ref_pad_bottom = -min(pad_bottom, 0)
+            ref_pad_right = -min(pad_right, 0)
+            self.report.log_debug(f"reference screenshot needs padding to match actual image size. padding bottom: {ref_pad_bottom}, padding right: {ref_pad_right}")
+            padded_ref_image = self._pad_image(img_ref, ref_pad_bottom, ref_pad_right)
+        if pad_bottom > 0 or pad_right > 0:
+            act_pad_bottom = max(pad_bottom, 0)
+            act_pad_right = max(pad_right, 0)
+            self.report.log(f"screenshot needs padding to match expected image size. padding bottom: {act_pad_bottom}, padding right: {act_pad_right}")
+            padded_image = self._pad_image(img, act_pad_bottom, act_pad_right)
+        return padded_image, padded_ref_image
+
+    def _pad_image(self, img: np.ndarray, pad_bottom: int, pad_right: int) -> np.ndarray:
         pad_top, pad_left, pad_colors = 0, 0, 0
         red = (255, 0, 0, 255) if self._img_has_alpha(img) else (255, 0, 0)
-        padded = np.pad(
-                img,
-                ((pad_top, pad_bottom), (pad_left, pad_right), (pad_colors, pad_colors)),
-                "constant",
+        # This will fail with numpy>1.22
+        return np.pad(
+                array=img,
+                pad_width=((pad_top, pad_bottom), (pad_left, pad_right), (pad_colors, pad_colors)),
+                mode="constant",
                 constant_values=[(red, red), (red, red), (0, 0)]
         )
-        return padded
 
     def _compute_ssim_and_diff(
             self,

requirements.txt

@@ -1,7 +1,10 @@
 Appium-Python-Client==4.0.1
 getgauge>=0.4.2
-numexpr==2.10.1
-scikit-image==0.24.0
+numexpr==2.10.0
+# newer numpy versions have changed the way images are padded.
+# scikit-image and numexpr rely on numpy and can not be updated.
+numpy==1.22.0
+scikit-image==0.22.0
 selenium==4.23.0
 webcolors==24.6.0
 webdriver-manager==4.0.1

setup.py

@@ -21,8 +21,9 @@
     install_requires=[
         'Appium-Python-Client==4.0.1',
         'getgauge>=0.4.2',
-        'numexpr==2.10.1',
-        'scikit-image==0.24.0',
+        'numexpr==2.10.0',
+        'numpy==1.22.0',
+        'scikit-image==0.22.0',
         'selenium==4.23.0',
         'webcolors==24.6.0',
         'webdriver-manager==4.0.1',

tests/__init__.py

@@ -3,9 +3,10 @@
 # SPDX-License-Identifier: MIT
 #
 
-import os
+from pathlib import Path
 
-PROJECT_DIR = os.getcwd()
-TEST_DIR = os.path.join(PROJECT_DIR, "tests")
-TEST_RESOURCES_DIR = os.path.join(TEST_DIR, "resources")
-TEST_OUT_DIR = os.path.join(TEST_DIR, "out")
+_test_dir = Path(__file__).absolute().parent
+TEST_DIR = str(_test_dir)
+PROJECT_DIR = str(_test_dir.parent)
+TEST_RESOURCES_DIR = str(_test_dir.joinpath("resources"))
+TEST_OUT_DIR = str(_test_dir.joinpath("out"))

tests/test_images.py

@@ -3,83 +3,135 @@
 # SPDX-License-Identifier: MIT
 #
 
+import numpy as np
 import os
 import shutil
-import inspect
 import unittest
+from numpy import uint8
+from skimage import img_as_ubyte, io
 from unittest.mock import MagicMock
-from pathlib import Path
 from parameterized import parameterized
 
-from skimage import io
-
 from gauge_web_app_steps.images import Images
+from tests import TEST_RESOURCES_DIR, TEST_OUT_DIR
 
 
 class TestImages(unittest.TestCase):
 
     def setUp(self) -> None:
         self.test_instance = Images(MagicMock())
-        # absolute path to this test class as a reference
-        path = Path(inspect.getfile(self.__class__))
-        absolute_path = path.parent.absolute()
-        self.resource = os.path.join(absolute_path, "resources")
-        self.out = os.path.join(absolute_path, "out")
-        self.actual_path = os.path.join(self.resource, "card1.png")
-        self.expected_path = os.path.join(self.resource, "card2.png")
+        self.actual_image = os.path.join(TEST_OUT_DIR, "actual_screenshots", "actual_rgba.png")
+        self.actual_image_rgb = os.path.join(TEST_OUT_DIR, "actual_screenshots", "actual_rgb.png")
+        self.expected_image = os.path.join(TEST_RESOURCES_DIR, "expected_rgba.png")
+        self.expected_image_rgb = os.path.join(TEST_RESOURCES_DIR, "expected_rgb.png")
+        self.diffs_dir = os.path.join(TEST_OUT_DIR, "diffs")
+        self.crop_dir = os.path.join(TEST_OUT_DIR, "crop")
+        os.makedirs(os.path.join(TEST_OUT_DIR, "actual_screenshots"), exist_ok=True)
+        os.makedirs(self.diffs_dir, exist_ok=True)
+        os.makedirs(self.crop_dir, exist_ok=True)
+        # the actual image file might be altered during the test, so the test runs on a copy
+        shutil.copy(os.path.join(TEST_RESOURCES_DIR, "actual_rgb.png"), self.actual_image_rgb)
+        shutil.copy(os.path.join(TEST_RESOURCES_DIR, "actual_rgba.png"), self.actual_image)
 
     @parameterized.expand(["full", "gradient", "red", "color:fuchsia"])
-    def test_adapt_and_compare_images(self, diff_format):
-        merge = "card1_merged.png"
-        self._remove_output_image_if_exists(merge)
+    def test_adapt_and_compare_images(self, diff_format:str):
+        mergefile = os.path.join(self.diffs_dir, "actual_rgba_merged.png")
+        self._remove_image_if_it_exists(mergefile)
+        expected_diff_file = os.path.join(self.diffs_dir, f"actual_rgba_{diff_format.removeprefix('color:')}.png")
+        self._remove_image_if_it_exists(expected_diff_file)
         ssim = self.test_instance.adapt_and_compare_images(
-            expected_screenshot_full_path=self.expected_path,
-            actual_screenshot_full_path=self.actual_path,
-            output_path=self.out,
+            expected_screenshot_full_path=self.expected_image,
+            actual_screenshot_full_path=self.actual_image,
+            output_path=self.diffs_dir,
             diff_formats=diff_format)
         self.assertGreater(ssim, 0.9)
         self.assertLess(ssim, 1)
-        self.assertFalse(self._exists_output_image(merge))
+        self.assertFalse(os.path.exists(mergefile))
+        self.assertTrue(os.path.exists(expected_diff_file), f"{expected_diff_file} does not exist")
+
+    def test__align_alpha_channel_of_actual_image__no_change(self):
+        img_expected = io.imread(self.expected_image)
+        img_actual = io.imread(self.actual_image)
+        result = self.test_instance._align_alpha_channel_of_actual_image(img_expected, img_actual)
+        self.assertTrue(img_actual is result)
+
+    def test__align_alpha_channel_of_actual_image__add_alpha(self):
+        img_expected = io.imread(self.expected_image)
+        img_actual = io.imread(self.actual_image_rgb)
+        result = self.test_instance._align_alpha_channel_of_actual_image(img_expected, img_actual)
+        before_color_depth = img_actual.shape[2]
+        result_color_depth = result.shape[2]
+        self.assertEqual(3, before_color_depth)
+        self.assertEqual(4, result_color_depth)
+
+    def test__align_alpha_channel_of_actual_image__remove_alpha(self):
+        img_expected = io.imread(self.expected_image_rgb)
+        img_actual = io.imread(self.actual_image)
+        result = self.test_instance._align_alpha_channel_of_actual_image(img_expected, img_actual)
+        before_color_depth = img_actual.shape[2]
+        result_color_depth = result.shape[2]
+        self.assertEqual(4, before_color_depth)
+        self.assertEqual(3, result_color_depth)
 
     def test_adapt_and_compare_images_merged(self):
-        merge = "card1_merged.png"
-        self._remove_output_image_if_exists(merge)
+        mergefile = os.path.join(self.diffs_dir, "actual_rgba_merged.png")
+        self._remove_image_if_it_exists(mergefile)
         ssim = self.test_instance.adapt_and_compare_images(
-            expected_screenshot_full_path=self.expected_path,
-            actual_screenshot_full_path=self.actual_path,
-            output_path=self.out,
+            expected_screenshot_full_path=self.expected_image,
+            actual_screenshot_full_path=self.actual_image,
+            output_path=self.diffs_dir,
             diff_formats="color:green",
             append_images=True)
         self.assertGreater(ssim, 0.9)
         self.assertLess(ssim, 1)
-        self.assertTrue(self._exists_output_image(merge))
-        self.assertFalse(self._exists_output_image("card1_green.png"))
+        self.assertTrue(os.path.exists(mergefile))
+        diff_file = os.path.join(self.diffs_dir, "actual_rgba_green.png")
+        self.assertFalse(os.path.exists(diff_file))
 
     def test_crop_image_file(self):
-        file = self._prepare_test_image()
+        file = self._prepare_crop_image()
         self.test_instance.crop_image_file(file,
             location={"x": 10, "y": 10},
             size={"width": 20, "height": 20},
             pixel_ratio=2, viewport_offset=10)
         img = io.imread(file)
         self.assertEqual((40, 40, 4), img.shape)
 
-    def _prepare_test_image(self):
-        # ensure that output folder exists
-        if not os.path.exists(self.out):
-            os.makedirs(self.out)
-        # copy a test resource there for further operations
-        shutil.copy(self.actual_path, self.out)
-        return os.path.join(self.out, "card1.png")
+    def test__pad_images__pad_actual(self):
+        img_actual = self._create_img(3, 3)
+        img_expected = self._create_img(4, 4)
+        img_actual_padded, img_expected_padded = self.test_instance._pad_images(img_actual, img_expected)
+        self.assertTrue(img_expected is img_expected_padded)
+        self.assertTupleEqual((4, 4, 4), img_actual_padded.shape)
+        padded_color = img_actual_padded[3][3]
+        expected_color = [255, 0, 0, 255]
+        self.assertListEqual(expected_color, padded_color.tolist())
+
+    def test__pad_images__pad_expected(self):
+        img_actual = self._create_img(4, 4)
+        img_expected = self._create_img(3, 3)
+        img_actual_padded, img_expected_padded = self.test_instance._pad_images(img_actual, img_expected)
+        self.assertTrue(img_actual is img_actual_padded)
+        self.assertTupleEqual((4, 4, 4), img_expected_padded.shape)
+        padded_color = img_expected_padded[3][3]
+        expected_color = [255, 0, 0, 255]
+        self.assertListEqual(expected_color, padded_color.tolist())
 
-    def _remove_output_image_if_exists(self, file_name):
-        if self._exists_output_image(file_name):
-            os.remove(os.path.join(self.out, file_name))
+    def _create_img(self, width: int, height: int) -> np.ndarray:
+        color = [215, 215, 215, 255]
+        width = [color for _ in range(width)]
+        img = np.array([width for _ in range(height)], dtype=uint8)
+        return img_as_ubyte(img)
 
-    def _exists_output_image(self, file_name):
-        path = os.path.join(self.out, file_name)
-        return os.path.exists(path)
+    def _prepare_crop_image(self):
+        # copy a test resource there for further operations
+        target = os.path.join(self.crop_dir, "crop.png")
+        shutil.copy(self.actual_image, target)
+        return target
 
+    def _remove_image_if_it_exists(self, file_path):
+        if os.path.exists(file_path):
+            os.remove(file_path)
 
 if __name__ == '__main__':
     unittest.main()