diff --git a/.gitignore b/.gitignore index f684bbaab..402d01211 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ /.projectile-cache.eld .claude/ AGENTS.md +dev/function-coverage.txt diff --git a/dev/buffer-leaks.el b/dev/buffer-leaks.el new file mode 100644 index 000000000..e119e4090 --- /dev/null +++ b/dev/buffer-leaks.el @@ -0,0 +1,61 @@ +;;; buffer-leaks.el --- Find specs that leave a foreign buffer current -*- lexical-binding: t; -*- + +;; Copyright © 2011-2026 Bozhidar Batsov + +;; This file is NOT part of GNU Emacs. + +;; This program is free software: you can redistribute it and/or +;; modify it under the terms of the GNU General Public License as +;; published by the Free Software Foundation, either version 3 of the +;; License, or (at your option) any later version. +;; +;; This program is distributed in the hope that it will be useful, but +;; WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +;; General Public License for more details. +;; +;; You should have received a copy of the GNU General Public License +;; along with this program. If not, see `http://www.gnu.org/licenses/'. + +;;; Commentary: + +;; A spec that calls `set-buffer' outside `save-current-buffer' leaves that +;; buffer current for everything that runs after it. Most specs don't care, +;; which is why such a leak sits unnoticed - until one that reads ambient +;; buffer state runs next and fails, on one Emacs version and not another. +;; That is not hypothetical: it is how `projectile-default-compilation-command' +;; (which asserts on `(point)') started failing on Emacs 30.2 alone. +;; +;; Usage, from the repository root: +;; +;; eldev -S "(load \"$PWD/dev/buffer-leaks.el\")" -p -dtT -C test +;; +;; Each offending spec is printed as a LEAK line naming the buffer it left +;; behind. A leak is worth fixing even when nothing currently trips over it, +;; because what trips over it is decided by spec order and by which buffers +;; the cleanup happens to kill. + +;;; Code: + +;; Buttercup is a test-time dependency, not a build one, so its functions are +;; not known when this file is byte-compiled. +(declare-function buttercup-spec-full-name "buttercup" (spec)) + +(defun projectile-buffer-leaks--check (orig spec &rest args) + "Run SPEC via ORIG with ARGS, reporting if it changes the current buffer." + (let ((before (current-buffer))) + (prog1 (apply orig spec args) + (unless (eq before (current-buffer)) + (princ (format "LEAK\t%s\t-> %s\n" + (buttercup-spec-full-name spec) + (buffer-name (current-buffer)))))))) + +(defun projectile-buffer-leaks--arm (&rest _) + "Wrap every spec once the suite is about to run." + (advice-add 'buttercup--run-spec :around #'projectile-buffer-leaks--check)) + +(advice-add 'buttercup-run :before #'projectile-buffer-leaks--arm) + +(provide 'buffer-leaks) + +;;; buffer-leaks.el ends here diff --git a/dev/function-coverage.el b/dev/function-coverage.el new file mode 100644 index 000000000..6baa8b7f1 --- /dev/null +++ b/dev/function-coverage.el @@ -0,0 +1,112 @@ +;;; function-coverage.el --- Which Projectile functions the suite never calls -*- lexical-binding: t; -*- + +;; Copyright © 2011-2026 Bozhidar Batsov + +;; This file is NOT part of GNU Emacs. + +;; This program is free software: you can redistribute it and/or +;; modify it under the terms of the GNU General Public License as +;; published by the Free Software Foundation, either version 3 of the +;; License, or (at your option) any later version. +;; +;; This program is distributed in the hope that it will be useful, but +;; WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +;; General Public License for more details. +;; +;; You should have received a copy of the GNU General Public License +;; along with this program. If not, see `http://www.gnu.org/licenses/'. + +;;; Commentary: + +;; Function-level coverage for the test suite, with no external dependency. +;; Every `defun' in projectile.el is advised to record that it ran, the suite +;; is run, and whatever was never called is reported. +;; +;; Usage, from the repository root: +;; +;; eldev -S "(load \"$PWD/dev/function-coverage.el\")" -p -dtT -C test +;; +;; The path has to be absolute: `-S' forms are evaluated before Eldev settles +;; on the project directory, so a relative one is not found. +;; +;; It writes `dev/function-coverage.txt' - one `HIT'/`MISS' line per function - +;; and prints a summary. Point `projectile-coverage-output' elsewhere to keep +;; the file out of the working tree. +;; +;; What it is and is not: this says whether a function was ever entered, not +;; which of its branches were. A `MISS' is therefore a hard fact - nothing +;; exercises that code at all - while a `HIT' only means something reached it. +;; Read the MISS list, not the percentage; the useful property is that the list +;; shrinks. +;; +;; One caveat worth knowing: a `defsubst' inlined into its callers at +;; compile time can be reported as a MISS even though its body runs. +;; projectile.el has one, so this is noise rather than a problem. + +;;; Code: + +(defvar projectile-coverage-source + (expand-file-name "projectile.el" + (file-name-directory + (directory-file-name + (file-name-directory (or load-file-name buffer-file-name))))) + "The file whose `defun's are watched.") + +(defvar projectile-coverage-output + (expand-file-name "function-coverage.txt" + (file-name-directory (or load-file-name buffer-file-name))) + "Where the HIT/MISS report is written.") + +(defvar projectile-coverage--called (make-hash-table :test 'eq) + "Functions that ran at least once during the suite.") + +(defvar projectile-coverage--watched nil + "Every function being watched, in definition order.") + +(defun projectile-coverage--instrument (&rest _) + "Advise every `defun' of `projectile-coverage-source' to record that it ran. +Idempotent, so it can be attached to something that runs more than once." + (unless projectile-coverage--watched + (with-temp-buffer + (insert-file-contents projectile-coverage-source) + (goto-char (point-min)) + (while (re-search-forward "^(defun \\([^ ()\n]+\\)" nil t) + (push (intern (match-string 1)) projectile-coverage--watched))) + (setq projectile-coverage--watched (nreverse projectile-coverage--watched)) + (dolist (fn projectile-coverage--watched) + ;; A macro or special form cannot be advised, and neither can a name + ;; that never got defined (a `defun' inside a `when' that was false). + (when (and (fboundp fn) (not (macrop fn)) (not (special-form-p fn))) + (let ((watched fn)) + (ignore-errors + (advice-add watched :before + (lambda (&rest _) + (puthash watched t projectile-coverage--called))))))) + (message "function-coverage: watching %d functions" + (length projectile-coverage--watched)))) + +(defun projectile-coverage--report (&rest _) + "Write the HIT/MISS report and print a one-line summary." + (when projectile-coverage--watched + (let ((hit 0) (missed 0)) + (with-temp-file projectile-coverage-output + (dolist (fn (sort (copy-sequence projectile-coverage--watched) + (lambda (a b) (string< (symbol-name a) (symbol-name b))))) + (if (gethash fn projectile-coverage--called) + (progn (setq hit (1+ hit)) (insert (format "HIT\t%s\n" fn))) + (setq missed (1+ missed)) + (insert (format "MISS\t%s\n" fn))))) + (message "function-coverage: %d/%d called (%.1f%%), %d never called - see %s" + hit (+ hit missed) + (if (> (+ hit missed) 0) (* 100.0 (/ (float hit) (+ hit missed))) 0.0) + missed projectile-coverage-output)))) + +;; `buttercup-run' is the last thing that happens before the specs, by which +;; point projectile and every test file have been loaded. +(advice-add 'buttercup-run :before #'projectile-coverage--instrument) +(add-hook 'kill-emacs-hook #'projectile-coverage--report) + +(provide 'function-coverage) + +;;; function-coverage.el ends here diff --git a/test/projectile-buffers-test.el b/test/projectile-buffers-test.el index 1c89b9c3f..f590fa91a 100644 --- a/test/projectile-buffers-test.el +++ b/test/projectile-buffers-test.el @@ -233,4 +233,67 @@ projectile-process-current-project-buffers-current to have similar behaviour" ;; A bunch of tests that make sure Projectile commands handle ;; gracefully the case of being run outside of a project. +;;; The buffer commands themselves + +(describe "projectile-kill-buffers" + (it "kills the project's buffers once confirmed" + (projectile-test-with-project + (("a.txt" . "x") ("b.txt" . "y")) + (let ((ba (find-file-noselect (expand-file-name "a.txt"))) + (bb (find-file-noselect (expand-file-name "b.txt")))) + (spy-on 'yes-or-no-p :and-return-value t) + (projectile-kill-buffers) + (expect (buffer-live-p ba) :to-be nil) + (expect (buffer-live-p bb) :to-be nil)))) + + (it "kills nothing when the confirmation is declined" + (projectile-test-with-project + (("a.txt" . "x")) + (let ((ba (find-file-noselect (expand-file-name "a.txt")))) + (spy-on 'yes-or-no-p :and-return-value nil) + (projectile-kill-buffers) + (expect (buffer-live-p ba) :to-be-truthy)))) + + (it "counts the buffers it is about to kill in the prompt" + (projectile-test-with-project + (("a.txt" . "x") ("b.txt" . "y")) + (find-file-noselect (expand-file-name "a.txt")) + (find-file-noselect (expand-file-name "b.txt")) + (spy-on 'yes-or-no-p :and-return-value nil) + (projectile-kill-buffers) + (expect (car (spy-calls-args-for 'yes-or-no-p 0)) :to-match "kill 2 buffers")))) + +(describe "projectile-next-project-buffer" + (it "keeps calling next-buffer until it lands on another project buffer" + (projectile-test-with-project + (("a.txt" . "x") ("b.txt" . "y")) + (let* ((ba (find-file-noselect (expand-file-name "a.txt"))) + (bb (find-file-noselect (expand-file-name "b.txt"))) + (visited nil)) + (spy-on 'projectile-project-buffers :and-return-value (list ba bb)) + ;; `save-current-buffer' matters: the command works on whatever buffer + ;; is current, and leaving a foreign one current leaks into every spec + ;; that runs afterwards. + (save-current-buffer + (set-buffer ba) + ;; a stand-in for `next-buffer': step through a fixed rotation that + ;; passes an unrelated buffer before reaching the project's other one + (let ((rotation (list (get-buffer-create "*unrelated*") bb))) + (cl-letf (((symbol-function 'next-buffer) + (lambda (&rest _) + (let ((next (or (pop rotation) bb))) + (push next visited) + (set-buffer next))))) + (projectile-next-project-buffer))) + (expect (current-buffer) :to-be bb)) + (kill-buffer "*unrelated*") + ;; it did not stop at the unrelated buffer on the way + (expect (length visited) :to-equal 2)))) + + (it "falls back to plain next-buffer outside a project" + (spy-on 'projectile-project-root :and-return-value nil) + (spy-on 'next-buffer) + (projectile-next-project-buffer) + (expect 'next-buffer :to-have-been-called))) + ;;; projectile-buffers-test.el ends here diff --git a/test/projectile-commands-test.el b/test/projectile-commands-test.el index 354006994..6eb34268a 100644 --- a/test/projectile-commands-test.el +++ b/test/projectile-commands-test.el @@ -1218,4 +1218,87 @@ (let ((projectile-use-comint-mode nil)) (expect (projectile-use-comint-mode-p 'task) :to-be nil)))) +;;; The commands themselves, not the machinery under them + +;; Every layer beneath these was covered and the commands were not, which is +;; how two wiring bugs shipped in one month: `s-p F' completing over an empty +;; list because it read a variable instead of its accessor, and +;; find-file-in-subproject offering nothing in a Go workspace. One spec each, +;; asking only whether the command reaches its destination. + +(describe "projectile-find-file" + (it "opens the chosen file, resolved against the project root" + (projectile-test-with-project + (("src/a.txt" . "x") ("b.txt" . "y")) + (let ((root default-directory)) + (spy-on 'projectile-completing-read :and-return-value "src/a.txt") + (spy-on 'find-file) + (projectile-find-file) + (expect 'find-file :to-have-been-called-with + (expand-file-name "src/a.txt" root))))) + + (it "offers the project's files and runs the find-file hook" + (projectile-test-with-project + (("src/a.txt" . "x") ("b.txt" . "y")) + (let ((ran nil)) + (spy-on 'projectile-completing-read :and-return-value "b.txt") + (spy-on 'find-file) + (let ((projectile-find-file-hook (list (lambda () (setq ran t))))) + (projectile-find-file)) + (expect ran :to-be-truthy) + (let ((offered (cadr (spy-calls-args-for 'projectile-completing-read 0)))) + (expect offered :to-contain "src/a.txt") + (expect offered :to-contain "b.txt"))))) + + (it "does nothing when the completion is declined" + (projectile-test-with-project + (("a.txt" . "x")) + (spy-on 'projectile-completing-read :and-return-value nil) + (spy-on 'find-file) + (projectile-find-file) + (expect 'find-file :not :to-have-been-called))) + + (it "invalidates the cache first when given a prefix argument" + (projectile-test-with-project + (("a.txt" . "x")) + (spy-on 'projectile-completing-read :and-return-value "a.txt") + (spy-on 'find-file) + (spy-on 'projectile-invalidate-cache) + (let ((projectile-enable-caching t)) + (projectile-find-file '(4))) + (expect 'projectile-invalidate-cache :to-have-been-called)))) + +(describe "projectile-find-file-all" + (it "offers what the generic listing command returns, ignore rules and all" + (projectile-test-with-project + ((".projectile" . "-/vendor\n") + ("keep.txt" . "x") + ("vendor/hidden.txt" . "y")) + (let ((root default-directory)) + (spy-on 'projectile-files-via-ext-command + :and-return-value '("keep.txt" "vendor/hidden.txt")) + (spy-on 'projectile-completing-read :and-return-value "vendor/hidden.txt") + (spy-on 'find-file) + (projectile-find-file-all) + ;; the ignored file is reachable - that is the point of the command + (expect (cadr (spy-calls-args-for 'projectile-completing-read 0)) + :to-contain "vendor/hidden.txt") + (expect 'find-file :to-have-been-called-with + (expand-file-name "vendor/hidden.txt" root)))))) + +(describe "projectile-switch-open-project" + (it "offers the open projects and switches to the chosen one" + (spy-on 'projectile-relevant-open-projects :and-return-value '("/a/" "/b/")) + (spy-on 'projectile-switch-project-by-name) + (spy-on 'projectile-completing-read :and-call-fake + (lambda (_prompt _choices &rest args) + (funcall (plist-get args :action) "/b/"))) + (projectile-switch-open-project) + (expect 'projectile-switch-project-by-name + :to-have-been-called-with "/b/" nil)) + + (it "says so when nothing is open" + (spy-on 'projectile-relevant-open-projects :and-return-value nil) + (expect (projectile-switch-open-project) :to-throw 'user-error))) + ;;; projectile-commands-test.el ends here diff --git a/test/projectile-replace-review-test.el b/test/projectile-replace-review-test.el index 34cac7fab..bf1e850b9 100644 --- a/test/projectile-replace-review-test.el +++ b/test/projectile-replace-review-test.el @@ -869,4 +869,122 @@ REGEXP-P selects `projectile-replace-regexp-review'." (expect (lookup-key projectile-command-map (kbd "u")) :to-be #'projectile-replace-undo))) +;;; The keys the results buffer is driven with + +(describe "the results buffer's own commands" + ;; The reviewer's machinery is covered exhaustively elsewhere; these are the + ;; commands the keymap actually invokes, which nothing was exercising. + + (it "toggles a single match in and out of the batch" + (projectile-test-with-project + (("a.txt" . "foo one\nfoo two\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (goto-char (point-min)) + (projectile-replace--goto-next-match) + (let ((m (projectile-replace--match-at-point))) + (expect (projectile-replace--match-enabled m) :to-be-truthy) + (projectile-replace--toggle) + (expect (projectile-replace--match-enabled m) :to-be nil) + (projectile-replace--toggle) + (expect (projectile-replace--match-enabled m) :to-be-truthy)))))) + + (it "refuses to toggle when point is not on a match" + (projectile-test-with-project + (("a.txt" . "foo one\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (goto-char (point-min)) ; the header, not a match + (expect (projectile-replace--toggle) :to-throw 'user-error))))) + + (it "toggles a whole file at once, and back" + (projectile-test-with-project + (("a.txt" . "foo one\nfoo two\n") + ("b.txt" . "foo three\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (goto-char (point-min)) + (projectile-replace--goto-next-match) + (let* ((file (projectile-replace--match-file + (projectile-replace--match-at-point))) + (of-file (lambda () + (cl-remove-if-not + (lambda (m) (equal (projectile-replace--match-file m) file)) + projectile-replace--matches)))) + (expect (length (funcall of-file)) :to-equal 2) + ;; any enabled -> all disabled + (projectile-replace--toggle-file) + (expect (cl-some #'projectile-replace--match-enabled (funcall of-file)) + :to-be nil) + ;; none enabled -> all enabled + (projectile-replace--toggle-file) + (expect (cl-every #'projectile-replace--match-enabled (funcall of-file)) + :to-be-truthy) + ;; and the other file is untouched + (expect (cl-every #'projectile-replace--match-enabled + projectile-replace--matches) + :to-be-truthy)))))) + + (it "walks between files and back between matches" + (projectile-test-with-project + (("a.txt" . "foo one\nfoo two\n") + ("b.txt" . "foo three\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (goto-char (point-min)) + (projectile-replace--goto-next-file) + (let ((first-file (get-text-property (line-beginning-position) + 'projectile-replace-file))) + (expect first-file :to-be-truthy) + (projectile-replace--goto-next-file) + (expect (get-text-property (line-beginning-position) + 'projectile-replace-file) + :not :to-equal first-file) + ;; and back the other way + (projectile-replace--goto-prev-file) + (expect (get-text-property (line-beginning-position) + 'projectile-replace-file) + :to-equal first-file)))))) + + (it "says so rather than moving when there is no further match" + (projectile-test-with-project + (("a.txt" . "foo one\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (goto-char (point-min)) + (projectile-replace--goto-next-match) + (let ((here (point))) + (spy-on 'message) + (projectile-replace--goto-prev-match) + (expect (point) :to-equal here) + (expect 'message :to-have-been-called)))))) + + (it "re-reads the replacement and re-renders the previews" + (projectile-test-with-project + (("a.txt" . "foo one\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (expect (buffer-string) :to-match "bar") + (spy-on 'read-string :and-return-value "baz") + (projectile-replace--set-replacement) + (expect projectile-replace--replacement :to-equal "baz") + (expect (buffer-string) :to-match "baz"))))) + + (it "cancels an in-flight scan on quit" + (projectile-test-with-project + (("a.txt" . "foo one\n")) + (projectile-test-use-plain-grep) + (let ((buf (projectile-replace-review-test--run "foo" "bar"))) + (with-current-buffer buf + (setq projectile-replace--scanning t) + (cl-letf (((symbol-function 'quit-window) #'ignore)) + (projectile-replace--quit)) + (expect projectile-replace--scanning :to-be nil)))))) + ;;; projectile-replace-review-test.el ends here diff --git a/test/projectile-search-review-test.el b/test/projectile-search-review-test.el index 73193b555..71304295f 100644 --- a/test/projectile-search-review-test.el +++ b/test/projectile-search-review-test.el @@ -559,36 +559,44 @@ REGEXP-P selects `projectile-search-regexp-review'." ;; The results buffer is redrawn from scratch, so a redraw costs time ;; proportional to the matches found so far; drawing on every chunk of a ;; streaming scan is most of the run on a large search. + ;; `before-each' runs outside any `save-current-buffer', so seeding the + ;; results buffer here would leave it current for every spec that follows. + ;; Hand it to the specs instead and let them enter it. + :var (buf) (before-each - (set-buffer (get-buffer-create projectile-search-buffer-name)) - (projectile-replace--seed (current-buffer) #'projectile-search-mode + (setq buf (get-buffer-create projectile-search-buffer-name)) + (projectile-replace--seed buf #'projectile-search-mode default-directory "foo" "foo" nil t t) (spy-on 'projectile-search--render)) (it "draws the first time, since a fresh buffer has never been drawn" - (let ((projectile-search-render-interval 3600)) - (projectile-replace--render-progress) - (expect 'projectile-search--render :to-have-been-called-times 1))) + (with-current-buffer buf + (let ((projectile-search-render-interval 3600)) + (projectile-replace--render-progress) + (expect 'projectile-search--render :to-have-been-called-times 1)))) (it "suppresses a redraw that comes too soon after the last" - (let ((projectile-search-render-interval 3600)) - (projectile-replace--render-progress) - (projectile-replace--render-progress) - (projectile-replace--render-progress) - (expect 'projectile-search--render :to-have-been-called-times 1))) + (with-current-buffer buf + (let ((projectile-search-render-interval 3600)) + (projectile-replace--render-progress) + (projectile-replace--render-progress) + (projectile-replace--render-progress) + (expect 'projectile-search--render :to-have-been-called-times 1)))) (it "draws again once the interval has passed" - (let ((projectile-search-render-interval 0.05)) - (projectile-replace--render-progress) - ;; rather than sleeping, age the buffer's last-draw stamp - (setq projectile-replace--last-render (- (float-time) 10)) - (projectile-replace--render-progress) - (expect 'projectile-search--render :to-have-been-called-times 2))) + (with-current-buffer buf + (let ((projectile-search-render-interval 0.05)) + (projectile-replace--render-progress) + ;; rather than sleeping, age the buffer's last-draw stamp + (setq projectile-replace--last-render (- (float-time) 10)) + (projectile-replace--render-progress) + (expect 'projectile-search--render :to-have-been-called-times 2)))) (it "draws on every call when the interval is nil" - (let ((projectile-search-render-interval nil)) - (dotimes (_ 5) (projectile-replace--render-progress)) - (expect 'projectile-search--render :to-have-been-called-times 5)))) + (with-current-buffer buf + (let ((projectile-search-render-interval nil)) + (dotimes (_ 5) (projectile-replace--render-progress)) + (expect 'projectile-search--render :to-have-been-called-times 5))))) (describe "point while results stream in" (it "keeps point where the user put it across a streaming redraw"