Remove last _needle in favor of ImageLogger

guibot/config.py

@@ -56,7 +56,6 @@ class GlobalConfig(type):
     _smooth_mouse_drag = True
     _screen_autoconnect = True
     _preprocess_special_chars = True
-    _save_needle_on_error = True
     _image_logging_level = logging.ERROR
     _image_logging_destination = "imglog"
     _image_logging_step_width = 3
@@ -195,26 +194,6 @@ def preprocess_special_chars(cls, value: bool = None) -> bool | None:
         fget=preprocess_special_chars, fset=preprocess_special_chars
     )
 
-    def save_needle_on_error(cls, value: bool = None) -> bool | None:
-        """
-        Getter/setter for property attribute.
-
-        :param value: whether to perform an extra needle dump on matching error
-        :returns: current value if no argument was passed otherwise None
-        """
-        if value is None:
-            return cls._save_needle_on_error
-        elif value is True or value is False:
-            cls._save_needle_on_error = value
-            return None
-        else:
-            raise ValueError
-
-    #: whether to perform an extra needle dump on matching error
-    save_needle_on_error = property(
-        fget=save_needle_on_error, fset=save_needle_on_error
-    )
-
     def image_logging_level(cls, value: int = None) -> int | None:
         """
         Getter/setter for property attribute.

guibot/imagelogger.py

@@ -25,6 +25,7 @@
 
 """
 
+import logging
 import os
 import shutil
 import PIL.Image
@@ -143,14 +144,14 @@ def draw_locations(
             x, y = loc
             cv2.circle(canvas, (int(x), int(y)), radius, (r, g, b))
 
-    def dump_matched_images(self) -> None:
+    def dump_matched_images(self, max_level: int = logging.WARNING) -> None:
         """
         Write file with the current needle and haystack.
 
         The current needle and haystack (matched images) are stored
         as `needle` and `haystack` attributes.
         """
-        if ImageLogger.logging_level > 30:
+        if ImageLogger.logging_level > max_level:
             return
         if not os.path.exists(ImageLogger.logging_destination):
             os.mkdir(ImageLogger.logging_destination)
@@ -169,6 +170,17 @@ def dump_matched_images(self) -> None:
         haystack_path = os.path.join(ImageLogger.logging_destination, haystack_name)
         self.haystack.save(haystack_path)
 
+    def dump_find_error(self) -> None:
+        """
+        Write the needle and haystack of a failed find.
+
+        This is a near minimal verbosity dump that is also active
+        at the default ERROR logging level.
+        """
+        self.dump_matched_images(max_level=logging.ERROR)
+        if ImageLogger.logging_level <= logging.ERROR:
+            ImageLogger.step += 1
+
     def dump_hotmap(self, name: str, hotmap: PIL.Image.Image | numpy.ndarray) -> None:
         """
         Write a file the given hotmap.

guibot/region.py

@@ -30,7 +30,6 @@
 """
 
 import time
-import os
 import logging
 
 # interconnected classes - carefully avoid circular reference
@@ -496,18 +495,13 @@ def find_all(
                 if allow_zero:
                     return last_matches
                 else:
-                    if GlobalConfig.save_needle_on_error is True:
-                        if not os.path.exists(ImageLogger.logging_destination):
-                            os.mkdir(ImageLogger.logging_destination)
-                        dump_path = GlobalConfig.image_logging_destination
-                        hdump_path = os.path.join(
-                            dump_path, "last_finderror_haystack.png"
-                        )
-                        ndump_path = os.path.join(
-                            dump_path, "last_finderror_needle.png"
-                        )
-                        screen_capture.save(hdump_path)
-                        target.save(ndump_path)
+                    imglog = ImageLogger()
+                    imglog.needle = target
+                    imglog.haystack = screen_capture
+                    try:
+                        imglog.dump_find_error()
+                    except Exception:
+                        log.exception("Failed to dump find-error images")
                     raise FindError(target)
 
             else:

tests/test_imagelogger.py

@@ -97,6 +97,16 @@ def test_image_dumping(self) -> None:
             self.imglog.needle.save.assert_called_once_with(os.path.join('imglog', 'imglog018-1needle-test_needle'))
             self.imglog.haystack.save.assert_called_once_with(os.path.join('imglog', 'imglog018-2haystack-test_haystack'))
 
+    def test_find_error_dumping(self) -> None:
+        """Check that a failed find dumps the needle and haystack."""
+        ImageLogger.step = 18
+        with patch("os.path.exists", side_effect=lambda _: False):
+            self.imglog.dump_find_error()
+            self.imglog.needle.save.assert_called_once_with(
+                os.path.join('imglog', 'imglog018-1needle-test_needle'))
+            self.imglog.haystack.save.assert_called_once_with(
+                os.path.join('imglog', 'imglog018-2haystack-test_haystack'))
+
     def test_hotmap_dumping(self) -> None:
         """Check that hotmaps are dumped correctly."""
         ImageLogger.step = 25

tests/test_region_expect.py

@@ -20,6 +20,7 @@
 import time
 import shutil
 import subprocess
+from unittest.mock import patch
 from typing import Any
 
 import common_test
@@ -174,6 +175,17 @@ def test_find_error(self) -> None:
         except FindError as e:
             pass
 
+        target = Image('shape_blue_circle.png')
+        with patch("guibot.region.ImageLogger") as logger_type:
+            self.assertRaises(FindError, self.region.find_all, target, 0)
+        logger = logger_type.return_value
+        self.assertIs(logger.needle, target)
+        logger.dump_find_error.assert_called_once_with()
+
+        with patch("guibot.region.ImageLogger") as logger_type:
+            logger_type.return_value.dump_find_error.side_effect = OSError("unwritable")
+            self.assertRaises(FindError, self.region.find_all, target, 0)
+
     @unittest.skipIf(os.environ.get('DISABLE_OPENCV', "0") == "1" or
                      os.environ.get('DISABLE_PYQT', "0") == "1",
                      "Disabled OpenCV or PyQt")