refactor(calendar-sync-source): drop dead fetch variant, test the sentinel

The buffer-string calendar-sync--fetch-ics had no callers. The sync path uses the temp-file variant exclusively, so 30 lines of curl and sentinel logic that would only drift are gone. I also pulled the fetch-ics-file sentinel's finish logic into calendar-sync--fetch-sentinel-finish so its success, failure, and temp-file-cleanup branches can be tested without a live curl process. The async-worker tests stub the whole fetch and never reached them.

2 files changed, 95 insertions, 46 deletions

diff --git a/modules/calendar-sync-source.el b/modules/calendar-sync-source.el
index 15c91c59..10dea66e 100644
--- a/modules/calendar-sync-source.el
+++ b/modules/calendar-sync-source.el
@@ -138,39 +138,25 @@ Checks `cj/debug-modules' for symbol `calendar-sync' or t (all)."
 
 ;;; .ics Fetch
 
-(defun calendar-sync--fetch-ics (url callback)
-  "Fetch .ics file from URL asynchronously using curl.
-Calls CALLBACK with the .ics content as string (normalized to Unix line endings)
-or nil on error. CALLBACK signature: (lambda (content) ...).
-
-The fetch happens asynchronously and doesn't block Emacs. The callback is
-invoked when the fetch completes, either successfully or with an error."
-  (condition-case err
-      (let ((buffer (generate-new-buffer " *calendar-sync-curl*")))
-        (make-process
-         :name "calendar-sync-curl"
-         :buffer buffer
-         :command (list "curl" "-s" "-L" "--fail"
-                        "--connect-timeout" "10"
-                        "--max-time" (number-to-string calendar-sync-fetch-timeout)
-                        url)
-         :sentinel
-         (lambda (process event)
-           (when (memq (process-status process) '(exit signal))
-             (let ((buf (process-buffer process)))
-               (when (buffer-live-p buf)
-                 (let ((content
-                        (with-current-buffer buf
-                          (if (and (eq (process-status process) 'exit)
-                                   (= (process-exit-status process) 0))
-                              (calendar-sync--normalize-line-endings (buffer-string))
-                            (calendar-sync--log-silently "calendar-sync: Fetch error: curl failed: %s" (string-trim event))
-                            nil))))
-                   (kill-buffer buf)
-                   (funcall callback content))))))))
-    (error
-     (calendar-sync--log-silently "calendar-sync: Fetch error: %s" (error-message-string err))
-     (funcall callback nil))))
+(defun calendar-sync--fetch-sentinel-finish (success event temp-file buffer callback)
+  "Finish an async .ics fetch.
+SUCCESS is non-nil when curl exited cleanly, EVENT the process event
+string, TEMP-FILE the curl output path, BUFFER the process buffer, and
+CALLBACK the continuation.  On success CALLBACK receives TEMP-FILE (the
+caller owns deleting it); on failure the error is logged, TEMP-FILE is
+removed, and CALLBACK receives nil.  Extracted from the sentinel so the
+success, failure, and cleanup branches are unit-testable without a live
+curl process."
+  (when (buffer-live-p buffer)
+    (unless success
+      (calendar-sync--log-silently "calendar-sync: Fetch error: curl failed: %s"
+                                   (string-trim event)))
+    (kill-buffer buffer))
+  (if success
+      (funcall callback temp-file)
+    (when (file-exists-p temp-file)
+      (delete-file temp-file))
+    (funcall callback nil)))
 
 (defun calendar-sync--fetch-ics-file (url callback)
   "Fetch .ics from URL to a temp file asynchronously.
@@ -190,19 +176,10 @@ owns deleting the temp file after a successful callback."
          :sentinel
          (lambda (process event)
            (when (memq (process-status process) '(exit signal))
-             (let ((buf (process-buffer process))
-                   (success (and (eq (process-status process) 'exit)
-                                 (= (process-exit-status process) 0))))
-               (when (buffer-live-p buf)
-                 (unless success
-                   (calendar-sync--log-silently "calendar-sync: Fetch error: curl failed: %s"
-                                                (string-trim event)))
-                 (kill-buffer buf))
-               (if success
-                   (funcall callback temp-file)
-                 (when (file-exists-p temp-file)
-                   (delete-file temp-file))
-                 (funcall callback nil)))))))
+             (calendar-sync--fetch-sentinel-finish
+              (and (eq (process-status process) 'exit)
+                   (= (process-exit-status process) 0))
+              event temp-file (process-buffer process) callback)))))
     (error
      (calendar-sync--log-silently "calendar-sync: Fetch error: %s" (error-message-string err))
      (funcall callback nil))))
diff --git a/tests/test-calendar-sync-source-fetch-sentinel.el b/tests/test-calendar-sync-source-fetch-sentinel.el
new file mode 100644
index 00000000..0b7ba1cf
--- /dev/null
+++ b/tests/test-calendar-sync-source-fetch-sentinel.el
@@ -0,0 +1,72 @@
+;;; test-calendar-sync-source-fetch-sentinel.el --- Tests for the fetch sentinel -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; calendar-sync--fetch-sentinel-finish is the extracted tail of the async
+;; .ics fetch sentinel.  The async-worker tests stub the whole fetch, so its
+;; success, failure, and temp-file-cleanup branches were never exercised.
+;; These tests drive the helper directly with fake success/failure inputs,
+;; no live curl process.
+
+;;; Code:
+
+(require 'ert)
+(require 'cl-lib)
+(require 'calendar-sync)
+
+(ert-deftest test-calendar-sync-fetch-sentinel-success-passes-temp-file ()
+  "Normal: on success the callback gets the temp file and it is not deleted."
+  (let ((temp-file (make-temp-file "calendar-sync-sentinel-" nil ".ics"))
+        (buffer (generate-new-buffer " *cs-test*"))
+        (got 'unset))
+    (unwind-protect
+        (progn
+          (calendar-sync--fetch-sentinel-finish
+           t "finished\n" temp-file buffer (lambda (r) (setq got r)))
+          (should (equal got temp-file))
+          (should (file-exists-p temp-file))
+          (should-not (buffer-live-p buffer)))
+      (when (file-exists-p temp-file) (delete-file temp-file))
+      (when (buffer-live-p buffer) (kill-buffer buffer)))))
+
+(ert-deftest test-calendar-sync-fetch-sentinel-failure-deletes-and-passes-nil ()
+  "Error: on failure the temp file is deleted and the callback gets nil."
+  (let ((temp-file (make-temp-file "calendar-sync-sentinel-" nil ".ics"))
+        (buffer (generate-new-buffer " *cs-test*"))
+        (got 'unset)
+        (logged nil))
+    (unwind-protect
+        (cl-letf (((symbol-function 'calendar-sync--log-silently)
+                   (lambda (&rest _) (setq logged t))))
+          (calendar-sync--fetch-sentinel-finish
+           nil "exited abnormally with code 22\n" temp-file buffer
+           (lambda (r) (setq got r)))
+          (should (null got))
+          (should-not (file-exists-p temp-file))
+          (should logged)
+          (should-not (buffer-live-p buffer)))
+      (when (file-exists-p temp-file) (delete-file temp-file))
+      (when (buffer-live-p buffer) (kill-buffer buffer)))))
+
+(ert-deftest test-calendar-sync-fetch-sentinel-failure-tolerates-missing-temp-file ()
+  "Boundary: failure with the temp file already gone does not error."
+  (let ((temp-file (make-temp-file "calendar-sync-sentinel-" nil ".ics"))
+        (got 'unset))
+    (delete-file temp-file)
+    (cl-letf (((symbol-function 'calendar-sync--log-silently) #'ignore))
+      (calendar-sync--fetch-sentinel-finish
+       nil "failed\n" temp-file nil (lambda (r) (setq got r)))
+      (should (null got)))))
+
+(ert-deftest test-calendar-sync-fetch-sentinel-tolerates-dead-buffer ()
+  "Boundary: a already-dead process buffer is not touched on success."
+  (let ((temp-file (make-temp-file "calendar-sync-sentinel-" nil ".ics"))
+        (got 'unset))
+    (unwind-protect
+        (progn
+          (calendar-sync--fetch-sentinel-finish
+           t "finished\n" temp-file nil (lambda (r) (setq got r)))
+          (should (equal got temp-file)))
+      (when (file-exists-p temp-file) (delete-file temp-file)))))
+
+(provide 'test-calendar-sync-source-fetch-sentinel)
+;;; test-calendar-sync-source-fetch-sentinel.el ends here