Improve error recording for macro cache generation

AddonCatalogCacheCreator.py

@@ -308,22 +308,31 @@ def generate_cache_entry_from_package_xml(
                         print(f"ERROR: Unknown error while reading icon file {absolute_icon_path}")
                         print(e)
                         icon_data_is_good = False
-                    if absolute_icon_path.lower().endswith(".svg"):
-                        try:
-                            if not icon_utils.is_svg_bytes(icon_data):
+                    if icon_data is not None:
+                        if absolute_icon_path.lower().endswith(".svg"):
+                            try:
+                                if not icon_utils.is_svg_bytes(icon_data):
+                                    self.icon_errors[metadata.name] = {
+                                        "valid_icon_path": relative_icon_path,
+                                        "error_message": "SVG file does not have valid XML header",
+                                    }
+                                    icon_data_is_good = False
+                            except icon_utils.BadIconData as e:
                                 self.icon_errors[metadata.name] = {
                                     "valid_icon_path": relative_icon_path,
-                                    "error_message": "SVG file does not have valid XML header",
+                                    "error_message": str(e),
                                 }
                                 icon_data_is_good = False
-                        except icon_utils.BadIconData as e:
-                            self.icon_errors[metadata.name] = {
-                                "valid_icon_path": relative_icon_path,
-                                "error_message": str(e),
-                            }
-                            icon_data_is_good = False
-                    if icon_data_is_good:
-                        cache_entry.icon_data = base64.b64encode(icon_data).decode("utf-8")
+                        elif absolute_icon_path.lower().endswith(".png"):
+                            if icon_utils.png_has_duplicate_iccp(icon_data):
+                                self.icon_errors[metadata.name] = {
+                                    "valid_icon_path": relative_icon_path,
+                                    "error_message": "PNG data has duplicate iCCP chunk",
+                                }
+                                icon_data_is_good = False
+
+                        if icon_data_is_good:
+                            cache_entry.icon_data = base64.b64encode(icon_data).decode("utf-8")
             else:
                 self.icon_errors[metadata.name] = {"bad_icon_path": relative_icon_path}
                 print(f"ERROR: Could not find icon file {absolute_icon_path}")

AddonManagerTest/app/test_freecad_interface.py

@@ -75,39 +75,34 @@ def test_log_no_freecad(self):
         """Test that if the FreeCAD import fails, the logger is set up correctly, and
         implements PrintLog"""
         sys.modules["FreeCAD"] = None
-        with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging:
-            import addonmanager_freecad_interface as fc
+        import addonmanager_freecad_interface as fc
 
+        with self.assertLogs("addonmanager", level="DEBUG") as cm:
             fc.Console.PrintLog("Test output")
-            self.assertTrue(isinstance(fc.Console, fc.ConsoleReplacement))
-            self.assertTrue(mock_logging.log.called)
 
     def test_message_no_freecad(self):
         """Test that if the FreeCAD import fails the logger implements PrintMessage"""
         sys.modules["FreeCAD"] = None
-        with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging:
-            import addonmanager_freecad_interface as fc
+        import addonmanager_freecad_interface as fc
 
+        with self.assertLogs("addonmanager", level="INFO") as cm:
             fc.Console.PrintMessage("Test output")
-            self.assertTrue(mock_logging.info.called)
 
     def test_warning_no_freecad(self):
         """Test that if the FreeCAD import fails the logger implements PrintWarning"""
         sys.modules["FreeCAD"] = None
-        with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging:
-            import addonmanager_freecad_interface as fc
+        import addonmanager_freecad_interface as fc
 
+        with self.assertLogs("addonmanager", level="WARNING") as cm:
             fc.Console.PrintWarning("Test output")
-            self.assertTrue(mock_logging.warning.called)
 
     def test_error_no_freecad(self):
         """Test that if the FreeCAD import fails the logger implements PrintError"""
         sys.modules["FreeCAD"] = None
-        with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging:
-            import addonmanager_freecad_interface as fc
+        import addonmanager_freecad_interface as fc
 
+        with self.assertLogs("addonmanager", level="ERROR") as cm:
             fc.Console.PrintError("Test output")
-            self.assertTrue(mock_logging.error.called)
 
 
 class TestParameters(WrapTestFreeCADImports):

AddonManagerTest/app/test_macro_cache_creator.py

@@ -23,6 +23,7 @@
 from unittest.mock import patch, MagicMock
 
 from MacroCacheCreator import MacroCatalog, CacheWriter
+import addonmanager_freecad_interface as fci
 
 
 class TestMacroCatalog(unittest.TestCase):
@@ -84,3 +85,30 @@ def test_retrieve_macros_fetch_failure(self, mock_console, mock_add_macro, mock_
         instance.retrieve_macros_from_wiki()
 
         mock_add_macro.assert_not_called()
+
+    def test_log_error(self):
+        instance = MacroCatalog()
+        instance.log_error("macro", "Test error")
+        instance.log_error("macro", "Test error 2")
+        self.assertIn("macro", instance.macro_errors)
+        self.assertEqual(len(instance.macro_errors["macro"]), 2)
+
+    def test_fetch_macros_logs_errors(self):
+
+        def fake_git(self):
+            fci.Console.PrintError("git failure")
+
+        def fake_wiki(self):
+            fci.Console.PrintWarning("wiki failure")
+
+        instance = MacroCatalog()
+        with patch.object(type(instance), "retrieve_macros_from_git", fake_git), patch.object(
+            type(instance), "retrieve_macros_from_wiki", fake_wiki
+        ):
+            instance.fetch_macros()
+
+        messages = [record["msg"] for record in instance.log_buffer]
+
+        self.assertIn("git failure", messages)
+        self.assertIn("wiki failure", messages)
+        self.assertEqual(len(messages), 2)

AddonManagerTest/gui/test_icon_utilities.py

@@ -3,9 +3,11 @@
 
 import gzip
 import io
+import struct
 import unittest
 from types import SimpleNamespace
 from unittest.mock import patch
+import zlib
 
 import addonmanager_icon_utilities as iu
 
@@ -435,3 +437,85 @@ def test_defaults_initialized_once_and_selected_by_repo_type(self):
         )
         a3 = iu.get_icon_for_addon(addon3)  # type: ignore[arg-type]
         self.assertIs(a3, iu.cached_default_icons["package"])
+
+
+def build_png_data(chunk_types: list[bytes]):
+
+    def chunk(typ, data):
+        return (
+            struct.pack(">I", len(data))
+            + typ
+            + data
+            + struct.pack(">I", zlib.crc32(typ + data) & 0xFFFFFFFF)
+        )
+
+    png = b"\x89PNG\r\n\x1a\n"
+
+    for chunk_type in chunk_types:
+        if chunk_type == b"IHDR":
+            # IHDR: 1x1 RGBA
+            ihdr = struct.pack(">IIBBBBB", 1, 1, 8, 6, 0, 0, 0)
+            png += chunk(b"IHDR", ihdr)
+
+        elif chunk_type == b"iCCP":
+            # iCCP: profile name + compression method + compressed ICC data
+            icc_profile = b"FakeICCProfile"
+            icc_compressed = zlib.compress(icc_profile)
+            iccp = b"icc\x00" + b"\x00" + icc_compressed
+            png += chunk(b"iCCP", iccp)
+
+        elif chunk_type == b"pHYs":
+            # 72 DPI
+            phys = struct.pack(">IIB", 2835, 2835, 1)
+            png += chunk(b"pHYs", phys)
+
+        elif chunk_type == b"tEXt":
+            text = b"Comment\x00PNG test file"
+            png += chunk(b"tEXt", text)
+
+        elif chunk_type == b"IDAT":
+            # IDAT: white pixel
+            raw = b"\x00\xff\xff\xff\xff"  # filter + RGBA
+            png += chunk(b"IDAT", zlib.compress(raw))
+
+        elif chunk_type == b"IEND":
+            # IEND
+            png += chunk(b"IEND", b"")
+
+    return png
+
+
+class TestPNGAnalysis(unittest.TestCase):
+
+    def test_get_chunk_types_simplest_possible_png(self):
+        simple_chunks = [b"IHDR", b"IDAT", b"IEND"]
+        png_data = build_png_data(simple_chunks)
+        chunks = iu.get_png_chunk_types(png_data)
+        self.assertEqual(chunks, simple_chunks)
+
+    def test_get_chunk_types_more_complete_png(self):
+        more_chunks = [b"IHDR", b"iCCP", b"pHYs", b"tEXt", b"IDAT", b"IEND"]
+        png_data = build_png_data(more_chunks)
+        chunks = iu.get_png_chunk_types(png_data)
+        self.assertEqual(chunks, more_chunks)
+
+    def test_get_chunk_types_extra_elements(self):
+        more_chunks = [b"IHDR", b"iCCP", b"iCCP", b"pHYs", b"tEXt", b"tEXt", b"IDAT", b"IEND"]
+        png_data = build_png_data(more_chunks)
+        chunks = iu.get_png_chunk_types(png_data)
+        self.assertEqual(chunks, more_chunks)
+
+    def test_valid_no_iccp(self):
+        simple_chunks = [b"IHDR", b"IDAT", b"IEND"]
+        png_data = build_png_data(simple_chunks)
+        self.assertFalse(iu.png_has_duplicate_iccp(png_data))
+
+    def test_good_iccp(self):
+        chunks_with_iccp = [b"IHDR", b"iCCP", b"pHYs", b"tEXt", b"IDAT", b"IEND"]
+        png_data = build_png_data(chunks_with_iccp)
+        self.assertFalse(iu.png_has_duplicate_iccp(png_data))
+
+    def test_duplicate_iccp(self):
+        chunks_with_iccp = [b"IHDR", b"iCCP", b"iCCP", b"pHYs", b"tEXt", b"IDAT", b"IEND"]
+        png_data = build_png_data(chunks_with_iccp)
+        self.assertTrue(iu.png_has_duplicate_iccp(png_data))