bastibe / org-journal

A simple org-mode based journaling mode
BSD 3-Clause "New" or "Revised" License
1.23k stars 122 forks source link

revise tests so they run cleanly on emacs 29.1 #412 #422

Closed jmay closed 6 months ago

TPKato commented 7 months ago

Using fset has many negative effects (in particular, running ert-run-tests-interactively is a disaster), so I think it is better to use another method. I don't know if it's the best, but cl-letf seems safer. Applying the following patch (after your PR #422) will fix this problem and all tests should pass (tested on emacs version 29.1).

diff --git a/tests/org-journal-test.el b/tests/org-journal-test.el
index 4e5b6e2..0b3baa9 100644
--- a/tests/org-journal-test.el
+++ b/tests/org-journal-test.el
@@ -227,10 +227,11 @@
         (save-buffer)
         (kill-buffer)
         (let ((message-marker nil))
-          (fset 'message (lambda (x y) (setq message-marker x)))
-          (org-journal-read-or-display-entry (encode-time 0 0 0 2 1 2019) 'noselect)
-          (should (equal "No journal entry for this date." message-marker))
-          ))))
+          (cl-letf (((symbol-function 'message)
+                     #'(lambda (x &rest y) (setq message-marker x))))
+            (org-journal-read-or-display-entry (encode-time 0 0 0 2 1 2019) 'noselect)
+            (should (equal "No journal entry for this date." message-marker))
+            )))))

 (ert-deftest org-journal-search-build-file-list-test ()
   "Test for `org-journal--search-build-file-list'."
@@ -348,7 +349,7 @@
                ;; "   "
                ;; org-scheduled-string
                ;; " "
-               (scheduled-string (concat "<" (format-time-string (cdr org-time-stamp-formats) scheduled-entry-time) ">")))
+               (scheduled-string (concat "SCHEDULED: " "<" (format-time-string (car org-time-stamp-formats) scheduled-entry-time) ">")))
           ;; Add first scheduled entry
           (org-journal-new-scheduled-entry nil scheduled-entry-time)
           (insert "Task 1")
bastibe commented 7 months ago

Thank you for working on this!

jmay commented 7 months ago

@TPKato Thanks for this. I applied your patch, but only the first hunk. The second hunk was causing the scheduled-weekly-test to fail due to the extra "SCHEDULED:" text. Perhaps this is necessary for #420 ? I don't want to merge these changes until they both pass independently.