From 2e334ec0258953d2a2c506ed61de9f8b4a739f4f Mon Sep 17 00:00:00 2001
From: Brian Pepple <bdpepple@gmail.com>
Date: Tue, 15 Oct 2024 16:32:44 -0400
Subject: [PATCH] If no files are listed in comic, don't attempted to remove
 the metadata

This should handle scenarios where the comic doesn't have the correct permissions.
---
 darkseid/comic.py   |  2 ++
 tests/test_comic.py | 40 ++++++++++++++++++++++++++++++++++------
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/darkseid/comic.py b/darkseid/comic.py
index 7e3b37a..913a79c 100644
--- a/darkseid/comic.py
+++ b/darkseid/comic.py
@@ -334,6 +334,8 @@ def remove_metadata(self: Comic) -> bool:
                 for path in self._archiver.get_filename_list()
                 if Path(path).name.lower() == self._ci_xml_filename.lower()
             ]
+            if not metadata_files:
+                return False
             write_success = self._archiver.remove_files(metadata_files)
             return self._successful_write(write_success, False, None)
         return True
diff --git a/tests/test_comic.py b/tests/test_comic.py
index 909898b..b3cc2d1 100644
--- a/tests/test_comic.py
+++ b/tests/test_comic.py
@@ -1,5 +1,6 @@
 # ruff: noqa: SLF001
 from pathlib import Path
+from unittest.mock import MagicMock
 
 import pytest
 
@@ -335,18 +336,45 @@ def test_write_metadata(mocker):
     assert result is True
 
 
-def test_remove_metadata(mocker):
+@pytest.mark.parametrize(
+    ("has_metadata", "filename_list", "ci_xml_filename", "remove_files_result", "expected"),
+    [
+        # Happy path: metadata present and successfully removed
+        (True, ["ComicInfo.xml"], "ComicInfo.xml", True, True),
+        # Happy path: metadata present but not removed
+        (True, ["ComicInfo.xml"], "ComicInfo.xml", False, False),
+        # Edge case: metadata not present in the archive
+        (True, ["otherfile.xml"], "ComicInfo.xml", True, False),
+        # Edge case: no files in the archive
+        (True, [], "ComicInfo.xml", True, False),
+        # Error case: has_metadata returns False
+        (False, ["ComicInfo.xml"], "ComicInfo.xml", True, True),
+    ],
+    ids=[
+        "metadata_present_and_removed",
+        "metadata_present_not_removed",
+        "metadata_not_present",
+        "no_files_in_archive",
+        "has_metadata_false",
+    ],
+)
+def test_remove_metadata(
+    has_metadata, filename_list, ci_xml_filename, remove_files_result, expected
+):
     # Arrange
-    comic = Comic("/path/to/comic.cbz")
-    mocker.patch.object(comic, "has_metadata", return_value=True)
-    mocker.patch.object(comic._archiver, "remove_file", return_value=True)
-    mocker.patch.object(comic, "_successful_write", return_value=True)
+    comic = Comic("bogus.cbz")
+    comic.has_metadata = MagicMock(return_value=has_metadata)
+    comic._archiver = MagicMock()
+    comic._archiver.get_filename_list = MagicMock(return_value=filename_list)
+    comic._archiver.remove_files = MagicMock(return_value=remove_files_result)
+    comic._ci_xml_filename = ci_xml_filename
+    comic._successful_write = MagicMock(return_value=remove_files_result)
 
     # Act
     result = comic.remove_metadata()
 
     # Assert
-    assert result is True
+    assert result == expected
 
 
 def test_remove_pages(mocker):