Added better exiftool error handling, closes #300

tests/Test-10.15.7.photoslibrary/database/Photos.sqlite.lock

@@ -7,7 +7,7 @@
 	<key>hostuuid</key>
 	<string>9575E48B-8D5F-5654-ABAC-4431B1167324</string>
 	<key>pid</key>
-	<integer>485</integer>
+	<integer>19275</integer>
 	<key>processname</key>
 	<string>photolibraryd</string>
 	<key>uid</key>

tests/test_catalina_10_15_7.py

@@ -18,10 +18,10 @@ PHOTOS_DB = "tests/Test-10.15.7.photoslibrary/database/photos.db"
 PHOTOS_DB_PATH = "/Test-10.15.7.photoslibrary/database/photos.db"
 PHOTOS_LIBRARY_PATH = "/Test-10.15.7.photoslibrary"
 
-PHOTOS_DB_LEN = 18
-PHOTOS_NOT_IN_TRASH_LEN = 16
+PHOTOS_DB_LEN = 19
+PHOTOS_NOT_IN_TRASH_LEN = 17
 PHOTOS_IN_TRASH_LEN = 2
-PHOTOS_DB_IMPORT_SESSIONS = 13
+PHOTOS_DB_IMPORT_SESSIONS = 14
 
 KEYWORDS = [
     "Kids",
@@ -971,7 +971,7 @@ def test_from_to_date(photosdb):
     time.tzset()
 
     photos = photosdb.photos(from_date=datetime.datetime(2018, 10, 28))
-    assert len(photos) == 9
+    assert len(photos) == 10 
 
     photos = photosdb.photos(to_date=datetime.datetime(2018, 10, 28))
     assert len(photos) == 7

tests/test_cli.py

@@ -405,6 +405,8 @@ CLI_EXIFTOOL_IGNORE_DATE_MODIFIED = {
     }
 }
 
+CLI_EXIFTOOL_ERROR = ["E2078879-A29C-4D6F-BACB-E3BBE6C3EB91"]
+
 LABELS_JSON = {
     "labels": {
         "Plant": 7,
@@ -1091,6 +1093,34 @@ def test_export_exiftool_quicktime():
             for filename in files:
                 os.unlink(filename)
 
+@pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
+def test_export_exiftool_error():
+    """" test --exiftool catching error """
+    import glob
+    import os
+    import os.path
+    from osxphotos.__main__ import export
+    from osxphotos.exiftool import ExifTool
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        for uuid in CLI_EXIFTOOL_ERROR:
+            result = runner.invoke(
+                export,
+                [
+                    os.path.join(cwd, PHOTOS_DB_15_7),
+                    ".",
+                    "-V",
+                    "--exiftool",
+                    "--uuid",
+                    f"{uuid}",
+                ],
+            )
+            assert result.exit_code == 0
+            assert "exiftool error" in result.output
+
 
 def test_export_edited_suffix():
     """ test export with --edited-suffix """

tests/test_exiftool.py

@@ -2,6 +2,7 @@ import pytest
 from osxphotos.exiftool import get_exiftool_path
 
 TEST_FILE_ONE_KEYWORD = "tests/test-images/wedding.jpg"
+TEST_FILE_BAD_IMAGE = "tests/test-images/badimage.jpeg"
 TEST_FILE_MULTI_KEYWORD = "tests/test-images/Tulips.jpg"
 TEST_MULTI_KEYWORDS = [
     "Top Shot",
@@ -109,8 +110,8 @@ def test_setvalue_1():
     assert exif.data["IPTC:Keywords"] == "test"
 
 
-def test_setvalue_error():
-    # test setting illegal tag value generates error
+def test_setvalue_warning():
+    # test setting illegal tag value generates warning
     import os.path
     import tempfile
     import osxphotos.exiftool
@@ -122,6 +123,22 @@ def test_setvalue_error():
 
     exif = osxphotos.exiftool.ExifTool(tempfile)
     exif.setvalue("IPTC:Foo", "test")
+    assert exif.warning
+
+
+def test_setvalue_error():
+    # test setting tag on bad image generates error
+    import os.path
+    import tempfile
+    import osxphotos.exiftool
+    from osxphotos.fileutil import FileUtil
+
+    tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    tempfile = os.path.join(tempdir.name, os.path.basename(TEST_FILE_BAD_IMAGE))
+    FileUtil.copy(TEST_FILE_BAD_IMAGE, tempfile)
+
+    exif = osxphotos.exiftool.ExifTool(tempfile)
+    exif.setvalue("IPTC:Keywords", "test")
     assert exif.error
 
 
@@ -142,7 +159,7 @@ def test_setvalue_context_manager():
         exif.setvalue("XMP:Title", "title")
         exif.setvalue("XMP:Subject", "subject")
 
-    assert exif.error is None
+    assert not exif.error
 
     exif2 = osxphotos.exiftool.ExifTool(tempfile)
     exif2._read_exif()
@@ -151,8 +168,8 @@ def test_setvalue_context_manager():
     assert exif2.data["XMP:Subject"] == "subject"
 
 
-def test_setvalue_context_manager_error():
-    # test setting a tag value as context manager when error generated
+def test_setvalue_context_manager_warning():
+    # test setting a tag value as context manager when warning generated
     import os.path
     import tempfile
     import osxphotos.exiftool
@@ -164,6 +181,22 @@ def test_setvalue_context_manager_error():
 
     with osxphotos.exiftool.ExifTool(tempfile) as exif:
         exif.setvalue("Foo:Bar", "test1")
+    assert exif.warning
+
+
+def test_setvalue_context_manager_error():
+    # test setting a tag value as context manager when error generated
+    import os.path
+    import tempfile
+    import osxphotos.exiftool
+    from osxphotos.fileutil import FileUtil
+
+    tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    tempfile = os.path.join(tempdir.name, os.path.basename(TEST_FILE_BAD_IMAGE))
+    FileUtil.copy(TEST_FILE_BAD_IMAGE, tempfile)
+
+    with osxphotos.exiftool.ExifTool(tempfile) as exif:
+        exif.setvalue("IPTC:Keywords", "test1")
     assert exif.error
 
 

tests/test_export_catalina_10_15_7.py

@@ -716,7 +716,7 @@ def test_xmp_sidecar_is_valid(tmp_path):
     xmp_file = tmp_path / XMP_FILENAME
     assert xmp_file.is_file()
     exiftool = ExifTool(str(xmp_file))
-    output, _ = exiftool.run_commands("-validate", "-warning")
+    output, _, _ = exiftool.run_commands("-validate", "-warning")
     assert output == b"[ExifTool]      Validate                        : 0 0 0"
 
 

tests/test_exportresults.py

@@ -0,0 +1,95 @@
+""" test ExportResults class """
+
+import pytest
+from osxphotos.photoinfo import ExportResults
+
+EXPORT_RESULT_ATTRIBUTES = [
+    "exported",
+    "new",
+    "updated",
+    "skipped",
+    "exif_updated",
+    "touched",
+    "converted_to_jpeg",
+    "sidecar_json_written",
+    "sidecar_json_skipped",
+    "sidecar_xmp_written",
+    "sidecar_xmp_skipped",
+    "missing",
+    "error",
+    "exiftool_warning",
+    "exiftool_error",
+]
+
+
+def test_exportresults_init():
+    results = ExportResults()
+    assert results.exported == []
+    assert results.new == []
+    assert results.updated == []
+    assert results.skipped == []
+    assert results.exif_updated == []
+    assert results.touched == []
+    assert results.converted_to_jpeg == []
+    assert results.sidecar_json_written == []
+    assert results.sidecar_json_skipped == []
+    assert results.sidecar_xmp_written == []
+    assert results.sidecar_xmp_skipped == []
+    assert results.missing == []
+    assert results.error == []
+    assert results.exiftool_warning == []
+    assert results.exiftool_error == []
+
+
+def test_exportresults_iadd():
+    results1 = ExportResults()
+    results2 = ExportResults()
+    for x in EXPORT_RESULT_ATTRIBUTES:
+        setattr(results1, x, [f"{x}1"])
+        setattr(results2, x, [f"{x}2"])
+
+    results1 += results2
+    for x in EXPORT_RESULT_ATTRIBUTES:
+        assert getattr(results1, x) == [f"{x}1", f"{x}2"]
+
+    # exiftool_warning and exiftool_error are lists of tuples
+    results1 = ExportResults()
+    results2 = ExportResults()
+    results1.exiftool_warning = [("exiftool_warning1", "foo")]
+    results2.exiftool_warning = [("exiftool_warning2", "bar")]
+    results1.exiftool_error = [("exiftool_error1", "foo")]
+    results2.exiftool_error = [("exiftool_error2", "bar")]
+
+    results1 += results2
+
+    assert results1.exiftool_warning == [
+        ("exiftool_warning1", "foo"),
+        ("exiftool_warning2", "bar"),
+    ]
+    assert results1.exiftool_error == [
+        ("exiftool_error1", "foo"),
+        ("exiftool_error2", "bar"),
+    ]
+
+
+def test_all_files():
+    """ test ExportResults.all_files() """
+    results = ExportResults()
+    for x in EXPORT_RESULT_ATTRIBUTES:
+        setattr(results, x, [f"{x}1"])
+    results.exiftool_warning = [("exiftool_warning1", "foo")]
+    results.exiftool_error = [("exiftool_error1", "foo")]
+
+    assert sorted(results.all_files()) == sorted(
+        [f"{x}1" for x in EXPORT_RESULT_ATTRIBUTES]
+    )
+
+
+def test_str():
+    """ test ExportResults.__str__ """
+    results = ExportResults()
+    assert (
+        str(results)
+        == "ExportResults(exported=[],new=[],updated=[],skipped=[],exif_updated=[],touched=[],converted_to_jpeg=[],sidecar_json_written=[],sidecar_json_skipped=[],sidecar_xmp_written=[],sidecar_xmp_skipped=[],missing=[],error=[],exiftool_warning=[],exiftool_error=[])"
+    )
+