fix(tests): add iCloud mocks to all test files and address code review feedback

- Add _should_upload_to_icloud mock to test_coverage_uploads_notification.py
  (_all_should_upload_false helper + 3 inline patch blocks)
- Add cfg.icloud_username/password = None to all onboarding test mocks
- Add iCloud creds to fully-configured onboarding test
- Fix noqa comment accuracy (unofficial third-party, not first-party)
- Replace generic Exception with RuntimeError in upload error handler

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>

app/tasks/upload_to_icloud.py

@@ -59,7 +59,7 @@ def _get_icloud_api(
         ImportError: If ``pyicloud`` is not installed.
         ValueError: If authentication fails or 2FA is required interactively.
     """
-    from pyicloud import PyiCloudService  # noqa: S404 – trusted first-party usage
+    from pyicloud import PyiCloudService  # noqa: S404 – unofficial third-party iCloud client
 
     kwargs: dict = {}
     if cookie_directory:
@@ -174,4 +174,4 @@ def upload_to_icloud(self, file_path: str, file_id: int = None, folder_override:
         error_msg = f"Error uploading {filename} to iCloud Drive: {e}"
         logger.error(f"[{task_id}] {error_msg}")
         log_task_progress(task_id, "upload_to_icloud", "failure", error_msg, file_id=file_id)
-        raise Exception(error_msg) from e
+        raise RuntimeError(error_msg) from e

tests/test_coverage_uploads_notification.py

@@ -665,6 +665,7 @@ def _all_should_upload_false():
         "email",
         "onedrive",
         "s3",
+        "icloud",
     ]
     return [patch(f"app.tasks.send_to_all._should_upload_to_{s}", return_value=False) for s in services]
 
@@ -693,6 +694,7 @@ class TestSendToAllCoverage:
             patch("app.tasks.send_to_all._should_upload_to_email", return_value=False),
             patch("app.tasks.send_to_all._should_upload_to_onedrive", return_value=False),
             patch("app.tasks.send_to_all._should_upload_to_s3", return_value=False),
+            patch("app.tasks.send_to_all._should_upload_to_icloud", return_value=False),
             patch("app.tasks.send_to_all.SessionLocal") as mock_session_cls,
         ):
             ms.workdir = str(tmp_path)
@@ -804,6 +806,7 @@ class TestSendToAllCoverage:
             patch("app.tasks.send_to_all._should_upload_to_email", return_value=False),
             patch("app.tasks.send_to_all._should_upload_to_onedrive", return_value=False),
             patch("app.tasks.send_to_all._should_upload_to_s3", return_value=False),
+            patch("app.tasks.send_to_all._should_upload_to_icloud", return_value=False),
             patch("app.tasks.send_to_all.SessionLocal"),
         ):
             ms.workdir = str(tmp_path)
@@ -863,6 +866,7 @@ class TestSendToAllCoverage:
             patch("app.tasks.send_to_all._should_upload_to_email", return_value=False),
             patch("app.tasks.send_to_all._should_upload_to_onedrive", return_value=False),
             patch("app.tasks.send_to_all._should_upload_to_s3", return_value=False),
+            patch("app.tasks.send_to_all._should_upload_to_icloud", return_value=False),
             patch("app.tasks.send_to_all.SessionLocal"),
         ):
             ms.workdir = str(tmp_path)

tests/test_views_onboarding.py

@@ -82,6 +82,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         assert result == []
@@ -107,6 +109,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -133,6 +137,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -159,6 +165,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -185,6 +193,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -211,6 +221,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -237,6 +249,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -263,6 +277,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -289,6 +305,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = "user"
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -315,6 +333,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = "ftp.example.com"
         cfg.ftp_username = "user"
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]
@@ -341,6 +361,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = "sftpuser"
         cfg.ftp_host = "ftp.example.com"
         cfg.ftp_username = "ftpuser"
+        cfg.icloud_username = "user@example.com"
+        cfg.icloud_password = "app-pass"
 
         result = _get_configured_destinations(cfg)
         assert len(result) == len(_DESTINATION_META)
@@ -366,6 +388,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         assert len(result) == 1
@@ -393,6 +417,8 @@ class TestGetConfiguredDestinations:
         cfg.sftp_username = None
         cfg.ftp_host = None
         cfg.ftp_username = None
+        cfg.icloud_username = None
+        cfg.icloud_password = None
 
         result = _get_configured_destinations(cfg)
         ids = [d["id"] for d in result]