refactor(reconcile): extract helpers, add recursive repo discovery and 28 tests

Extract should-skip-p, pull-clean, pull-dirty from 6-level nested
reconcile-git-directory. Make find-git-repos recurse into sub-repos.

6 files changed, 501 insertions, 0 deletions

diff --git a/tests/test-reconcile--find-git-repos.el b/tests/test-reconcile--find-git-repos.el
new file mode 100644
index 00000000..25987818
--- /dev/null
+++ b/tests/test-reconcile--find-git-repos.el
@@ -0,0 +1,77 @@
+;;; test-reconcile--find-git-repos.el --- Tests for cj/find-git-repos -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for recursive git repository discovery in cj/find-git-repos.
+;; Uses real temporary directory trees with fake .git directories.
+
+;;; Code:
+
+(require 'ert)
+(require 'testutil-reconcile-open-repos)
+(require 'reconcile-open-repos)
+
+;;; Normal Cases
+
+(ert-deftest test-find-git-repos-normal-flat-repos ()
+  "Finds multiple git repos at the same level."
+  (reconcile-test-with-temp-dirs
+   ("repo-a/.git/" "repo-b/.git/" "repo-c/.git/")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 3)))))
+
+(ert-deftest test-find-git-repos-normal-nested-repo ()
+  "Finds a repo nested inside a non-repo directory."
+  (reconcile-test-with-temp-dirs
+   ("parent/child/.git/")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 1))
+     (should (string-suffix-p "child" (car repos))))))
+
+(ert-deftest test-find-git-repos-normal-repo-with-nested-subrepo ()
+  "Finds both a parent repo and a sub-repo inside it."
+  (reconcile-test-with-temp-dirs
+   ("deepsat/.git/" "deepsat/frontend/.git/" "deepsat/backend/.git/")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 3)))))
+
+(ert-deftest test-find-git-repos-normal-mixed-repos-and-dirs ()
+  "Finds repos while skipping plain directories."
+  (reconcile-test-with-temp-dirs
+   ("repo-a/.git/" "not-a-repo/readme.txt" "repo-b/.git/")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 2)))))
+
+(ert-deftest test-find-git-repos-normal-deeply-nested ()
+  "Finds a repo several levels deep."
+  (reconcile-test-with-temp-dirs
+   ("a/b/c/deep-repo/.git/")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 1))
+     (should (string-suffix-p "deep-repo" (car repos))))))
+
+;;; Boundary Cases
+
+(ert-deftest test-find-git-repos-boundary-empty-directory ()
+  "Returns empty list for directory with no children."
+  (reconcile-test-with-temp-dirs
+   ()
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 0)))))
+
+(ert-deftest test-find-git-repos-boundary-no-git-repos ()
+  "Returns empty list when no directories contain .git."
+  (reconcile-test-with-temp-dirs
+   ("dir-a/file.txt" "dir-b/file.txt")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 0)))))
+
+(ert-deftest test-find-git-repos-boundary-hidden-dirs-skipped ()
+  "Skips hidden directories (starting with dot) per the regex filter."
+  (reconcile-test-with-temp-dirs
+   (".hidden-repo/.git/" "visible-repo/.git/")
+   (let ((repos (cj/find-git-repos test-root)))
+     (should (= (length repos) 1))
+     (should (string-suffix-p "visible-repo" (car repos))))))
+
+(provide 'test-reconcile--find-git-repos)
+;;; test-reconcile--find-git-repos.el ends here
diff --git a/tests/test-reconcile--git-directory.el b/tests/test-reconcile--git-directory.el
new file mode 100644
index 00000000..ab4a6323
--- /dev/null
+++ b/tests/test-reconcile--git-directory.el
@@ -0,0 +1,94 @@
+;;; test-reconcile--git-directory.el --- Tests for cj/reconcile-git-directory -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for the top-level reconcile function that dispatches to skip/clean/dirty.
+
+;;; Code:
+
+(require 'ert)
+(require 'testutil-reconcile-open-repos)
+(require 'reconcile-open-repos)
+
+;;; Normal Cases
+
+(ert-deftest test-reconcile-git-directory-normal-clean-repo-pulls ()
+  "Clean SSH repo calls pull-clean, not pull-dirty."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (clean-called nil)
+         (dirty-called nil))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (cond ((string-match-p "remote.origin.url" cmd) "git@host:repo.git")
+                 ((string-match-p "status --porcelain" cmd) "")
+                 (t "")))
+       (cl-letf (((symbol-function 'cj/reconcile--pull-clean)
+                  (lambda (_dir) (setq clean-called t)))
+                 ((symbol-function 'cj/reconcile--pull-dirty)
+                  (lambda (_dir) (setq dirty-called t)))
+                 ((symbol-function 'message) (lambda (_fmt &rest _args))))
+         (cj/reconcile-git-directory dir)))
+     (should clean-called)
+     (should-not dirty-called))))
+
+(ert-deftest test-reconcile-git-directory-normal-dirty-repo-stashes ()
+  "Dirty SSH repo calls pull-dirty, not pull-clean."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (clean-called nil)
+         (dirty-called nil))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (cond ((string-match-p "remote.origin.url" cmd) "git@host:repo.git")
+                 ((string-match-p "status --porcelain" cmd) " M file.el\n")
+                 (t "")))
+       (cl-letf (((symbol-function 'cj/reconcile--pull-clean)
+                  (lambda (_dir) (setq clean-called t)))
+                 ((symbol-function 'cj/reconcile--pull-dirty)
+                  (lambda (_dir) (setq dirty-called t)))
+                 ((symbol-function 'message) (lambda (_fmt &rest _args))))
+         (cj/reconcile-git-directory dir)))
+     (should-not clean-called)
+     (should dirty-called))))
+
+(ert-deftest test-reconcile-git-directory-normal-skipped-repo-no-calls ()
+  "HTTP repo is skipped entirely — neither pull-clean nor pull-dirty called."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (clean-called nil)
+         (dirty-called nil))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd)
+               "https://github.com/user/repo.git"
+             ""))
+       (cl-letf (((symbol-function 'cj/reconcile--pull-clean)
+                  (lambda (_dir) (setq clean-called t)))
+                 ((symbol-function 'cj/reconcile--pull-dirty)
+                  (lambda (_dir) (setq dirty-called t)))
+                 ((symbol-function 'message) (lambda (_fmt &rest _args))))
+         (cj/reconcile-git-directory dir)))
+     (should-not clean-called)
+     (should-not dirty-called))))
+
+;;; Boundary Cases
+
+(ert-deftest test-reconcile-git-directory-boundary-emits-checking-message ()
+  "Always emits 'checking: <dir>' message, even for skipped repos."
+  (reconcile-test-with-temp-dirs
+   ("repo/readme.txt")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (cl-letf (((symbol-function 'message)
+                (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+       (cj/reconcile-git-directory dir))
+     (should (cl-some (lambda (m) (string-match-p "checking:" m)) messages)))))
+
+(provide 'test-reconcile--git-directory)
+;;; test-reconcile--git-directory.el ends here
diff --git a/tests/test-reconcile--pull-clean.el b/tests/test-reconcile--pull-clean.el
new file mode 100644
index 00000000..a10c6f1e
--- /dev/null
+++ b/tests/test-reconcile--pull-clean.el
@@ -0,0 +1,60 @@
+;;; test-reconcile--pull-clean.el --- Tests for cj/reconcile--pull-clean -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for pulling latest changes on a clean git repository.
+
+;;; Code:
+
+(require 'ert)
+(require 'testutil-reconcile-open-repos)
+(require 'reconcile-open-repos)
+
+;;; Normal Cases
+
+(ert-deftest test-pull-clean-normal-success ()
+  "Successful pull produces no warning message."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (_cmd) "")
+       (cl-letf (((symbol-function 'message)
+                  (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+         (cj/reconcile--pull-clean dir)))
+     (should-not (cl-some (lambda (m) (string-match-p "Warning" m)) messages)))))
+
+(ert-deftest test-pull-clean-normal-failure-warns ()
+  "Failed pull produces a warning message with directory and exit code."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 1)
+         (lambda (_cmd) "")
+       (cl-letf (((symbol-function 'message)
+                  (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+         (cj/reconcile--pull-clean dir)))
+     (should (cl-some (lambda (m) (string-match-p "Warning.*git pull failed" m)) messages))
+     (should (cl-some (lambda (m) (string-match-p "exit code: 1" m)) messages)))))
+
+;;; Boundary Cases
+
+(ert-deftest test-pull-clean-boundary-nonzero-exit-128 ()
+  "Exit code 128 (common git error) is reported in warning."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 128)
+         (lambda (_cmd) "")
+       (cl-letf (((symbol-function 'message)
+                  (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+         (cj/reconcile--pull-clean dir)))
+     (should (cl-some (lambda (m) (string-match-p "exit code: 128" m)) messages)))))
+
+(provide 'test-reconcile--pull-clean)
+;;; test-reconcile--pull-clean.el ends here
diff --git a/tests/test-reconcile--pull-dirty.el b/tests/test-reconcile--pull-dirty.el
new file mode 100644
index 00000000..2ba1f5d1
--- /dev/null
+++ b/tests/test-reconcile--pull-dirty.el
@@ -0,0 +1,112 @@
+;;; test-reconcile--pull-dirty.el --- Tests for cj/reconcile--pull-dirty -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for the dirty-repo reconciliation: stash, pull, pop, magit.
+
+;;; Code:
+
+(require 'ert)
+(require 'testutil-reconcile-open-repos)
+(require 'reconcile-open-repos)
+
+;;; Normal Cases
+
+(ert-deftest test-pull-dirty-normal-stash-pull-pop-success ()
+  "When stash, pull, and pop all succeed, magit is still opened."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-magit-mock
+       (reconcile-test-with-shell-mocks
+           (lambda (_cmd) 0)
+           (lambda (_cmd) "")
+         (cj/reconcile--pull-dirty dir))
+       (should (member dir reconcile-test-magit-calls))))))
+
+(ert-deftest test-pull-dirty-normal-stash-fails-opens-magit ()
+  "When stash fails, magit is opened and warning emitted."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-magit-mock
+       (reconcile-test-with-shell-mocks
+           (lambda (cmd)
+             (if (string-match-p "stash --quiet\\'" cmd) 1 0))
+           (lambda (_cmd) "")
+         (cl-letf (((symbol-function 'message)
+                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+           (cj/reconcile--pull-dirty dir)))
+       (should (member dir reconcile-test-magit-calls))
+       (should (cl-some (lambda (m) (string-match-p "stash failed" m)) messages))))))
+
+(ert-deftest test-pull-dirty-normal-pull-fails-warns ()
+  "When stash succeeds but pull fails, warning mentions pull failure."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-magit-mock
+       (reconcile-test-with-shell-mocks
+           (lambda (cmd)
+             (cond ((string-match-p "stash --quiet\\'" cmd) 0)
+                   ((string-match-p "pull" cmd) 1)
+                   (t 0)))
+           (lambda (_cmd) "")
+         (cl-letf (((symbol-function 'message)
+                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+           (cj/reconcile--pull-dirty dir)))
+       (should (cl-some (lambda (m) (string-match-p "git pull failed" m)) messages))))))
+
+(ert-deftest test-pull-dirty-normal-stash-pop-fails-warns ()
+  "When stash and pull succeed but pop fails, warning mentions stash pop."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-magit-mock
+       (reconcile-test-with-shell-mocks
+           (lambda (cmd)
+             (cond ((string-match-p "stash pop" cmd) 1)
+                   ((string-match-p "stash" cmd) 0)
+                   (t 0)))
+           (lambda (_cmd) "")
+         (cl-letf (((symbol-function 'message)
+                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+           (cj/reconcile--pull-dirty dir)))
+       (should (cl-some (lambda (m) (string-match-p "stash pop failed" m)) messages))))))
+
+;;; Boundary Cases
+
+(ert-deftest test-pull-dirty-boundary-always-opens-magit ()
+  "Magit is opened regardless of whether pull succeeds or fails."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     ;; Test with pull failure
+     (reconcile-test-with-magit-mock
+       (reconcile-test-with-shell-mocks
+           (lambda (cmd)
+             (if (string-match-p "pull" cmd) 1 0))
+           (lambda (_cmd) "")
+         (cl-letf (((symbol-function 'message) (lambda (_fmt &rest _args))))
+           (cj/reconcile--pull-dirty dir)))
+       (should (member dir reconcile-test-magit-calls))))))
+
+(ert-deftest test-pull-dirty-boundary-uncommitted-work-message ()
+  "Always emits 'contains uncommitted work' message."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root))
+         (messages nil))
+     (reconcile-test-with-magit-mock
+       (reconcile-test-with-shell-mocks
+           (lambda (_cmd) 0)
+           (lambda (_cmd) "")
+         (cl-letf (((symbol-function 'message)
+                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
+           (cj/reconcile--pull-dirty dir)))
+       (should (cl-some (lambda (m) (string-match-p "uncommitted work" m)) messages))))))
+
+(provide 'test-reconcile--pull-dirty)
+;;; test-reconcile--pull-dirty.el ends here
diff --git a/tests/test-reconcile--should-skip-p.el b/tests/test-reconcile--should-skip-p.el
new file mode 100644
index 00000000..3e9c0177
--- /dev/null
+++ b/tests/test-reconcile--should-skip-p.el
@@ -0,0 +1,101 @@
+;;; test-reconcile--should-skip-p.el --- Tests for cj/reconcile--should-skip-p -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for the skip predicate that filters out non-git dirs, local-only repos,
+;; and http/https reference clones.
+
+;;; Code:
+
+(require 'ert)
+(require 'testutil-reconcile-open-repos)
+(require 'reconcile-open-repos)
+
+;;; Normal Cases
+
+(ert-deftest test-should-skip-p-normal-ssh-remote-not-skipped ()
+  "SSH remote repo should NOT be skipped."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd)
+               "git@github.com:user/repo.git"
+             ""))
+       (should-not (cj/reconcile--should-skip-p dir))))))
+
+(ert-deftest test-should-skip-p-normal-http-remote-skipped ()
+  "HTTP remote repo should be skipped (reference clone)."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd)
+               "http://github.com/user/repo.git"
+             ""))
+       (should (cj/reconcile--should-skip-p dir))))))
+
+(ert-deftest test-should-skip-p-normal-https-remote-skipped ()
+  "HTTPS remote repo should be skipped (reference clone)."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd)
+               "https://github.com/user/repo.git"
+             ""))
+       (should (cj/reconcile--should-skip-p dir))))))
+
+(ert-deftest test-should-skip-p-normal-no-remote-skipped ()
+  "Local-only repo (no remote) should be skipped."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd) "" ""))
+       (should (cj/reconcile--should-skip-p dir))))))
+
+;;; Boundary Cases
+
+(ert-deftest test-should-skip-p-boundary-no-git-dir ()
+  "Directory without .git should be skipped."
+  (reconcile-test-with-temp-dirs
+   ("not-a-repo/readme.txt")
+   (let ((dir (expand-file-name "not-a-repo" test-root)))
+     (should (cj/reconcile--should-skip-p dir)))))
+
+(ert-deftest test-should-skip-p-boundary-scp-style-remote-not-skipped ()
+  "SCP-style remote (user@host:path) should NOT be skipped."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd)
+               "user@myserver.com:repos/project.git"
+             ""))
+       (should-not (cj/reconcile--should-skip-p dir))))))
+
+(ert-deftest test-should-skip-p-boundary-ssh-protocol-url-not-skipped ()
+  "ssh:// protocol URL should NOT be skipped."
+  (reconcile-test-with-temp-dirs
+   ("repo/.git/")
+   (let ((dir (expand-file-name "repo" test-root)))
+     (reconcile-test-with-shell-mocks
+         (lambda (_cmd) 0)
+         (lambda (cmd)
+           (if (string-match-p "remote.origin.url" cmd)
+               "ssh://git@github.com/user/repo.git"
+             ""))
+       (should-not (cj/reconcile--should-skip-p dir))))))
+
+(provide 'test-reconcile--should-skip-p)
+;;; test-reconcile--should-skip-p.el ends here
diff --git a/tests/testutil-reconcile-open-repos.el b/tests/testutil-reconcile-open-repos.el
new file mode 100644
index 00000000..2d8614eb
--- /dev/null
+++ b/tests/testutil-reconcile-open-repos.el
@@ -0,0 +1,57 @@
+;;; testutil-reconcile-open-repos.el --- Test helpers for reconcile-open-repos -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Provides helper macros and functions for testing reconcile-open-repos.
+;; Creates temporary directory trees with fake .git dirs and mocks shell commands.
+
+;;; Code:
+
+(require 'cl-lib)
+
+(defmacro reconcile-test-with-temp-dirs (dir-spec &rest body)
+  "Create a temp directory tree per DIR-SPEC, bind `test-root', then run BODY.
+DIR-SPEC is a list of relative paths.  Paths ending in / create directories;
+others create files.  A path containing `.git/' creates the .git dir automatically.
+
+Example:
+  (reconcile-test-with-temp-dirs
+   (\"repo-a/.git/\" \"repo-b/subdir/\" \"not-a-repo/readme.txt\")
+   ...use test-root...)"
+  (declare (indent 1))
+  `(let ((test-root (make-temp-file "reconcile-test-" t)))
+     (unwind-protect
+         (progn
+           (dolist (path ',dir-spec)
+             (let ((full (expand-file-name path test-root)))
+               (if (string-suffix-p "/" path)
+                   (make-directory full t)
+                 (progn
+                   (make-directory (file-name-directory full) t)
+                   (write-region "" nil full)))))
+           ,@body)
+       (delete-directory test-root t))))
+
+(defmacro reconcile-test-with-shell-mocks (shell-cmd-fn shell-cmd-to-str-fn &rest body)
+  "Run BODY with `shell-command' and `shell-command-to-string' overridden.
+SHELL-CMD-FN receives (command) and returns an exit code integer.
+SHELL-CMD-TO-STR-FN receives (command) and returns a string."
+  (declare (indent 2))
+  `(cl-letf (((symbol-function 'shell-command)
+              (lambda (cmd &rest _) (funcall ,shell-cmd-fn cmd)))
+             ((symbol-function 'shell-command-to-string)
+              (lambda (cmd) (funcall ,shell-cmd-to-str-fn cmd))))
+     ,@body))
+
+(defvar reconcile-test-magit-calls nil
+  "List of directories passed to magit-status during tests.")
+
+(defmacro reconcile-test-with-magit-mock (&rest body)
+  "Run BODY with `magit-status' mocked to record calls."
+  (declare (indent 0))
+  `(let ((reconcile-test-magit-calls nil))
+     (cl-letf (((symbol-function 'magit-status)
+                (lambda (dir &rest _) (push dir reconcile-test-magit-calls))))
+       ,@body)))
+
+(provide 'testutil-reconcile-open-repos)
+;;; testutil-reconcile-open-repos.el ends here