Hello! I was trying to increase test-coverage of org-clock.el but immediately bumped into a couple of bugs so I didn't get very far.
Do note there is a FIXME in the final patch. bug #1: month addition in org-timestamp-change. In an org-mode buffer try to run `org-timestamp-up' (S-<up>) on the month of the timestamp "<2026-01-31 Sat>". It becomes "<2026-03-03 Tue>"! That's not right. We are not doing the month roll over calculations correctly. I'd rather not do these calculations by hand and would rather use the shiny function `decoded-time-add'. This is a wonderful function that I'd like to use more throughout the org-mode code base. Only problem is that until quite recently, it bugged out on negative month values so I'm forced to add a compatibility shim around it. bug #2: org-clock-timestamps-change likes to error In an org-buffer insert the following, then put point on the first bracket and run `org-clock-timestamps-up' (C-S-<up>). You will get a funny error and the brackets will turn to active brackets. CLOCK: [2026-03-15 Sun 15:55]--[2026-03-15 Sun 16:50] => 0:55 Other less significant changes you'll find in my patchset: Back in commit c81c844d1 you fixed the test `test-org/org-timestamp-change' by hard-coding the date. In the future I'm planning on running the test suite at specific times (maybe using libfaketime) to catch some bugs. Specifically some DST bugs I imagine. So I did my best to fix the test without hard coding the date. And the usual blurb: Tests pass after each patch on emacs 30.2. Tests pass after final patch on emacs 28, 29 and a recent emacs/master build. Tests pass after final patch with TZ set to UTC, Europe/Istanbul, and America/New_York.
>From 3d681baf30b6636c90e37d9f14721b7284814763 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sat, 14 Mar 2026 16:14:17 -0400 Subject: [PATCH 1/7] org-timestamp-change: Use proper time structure accessor functions * lisp/org.el (org-timestamp-change): Use proper accessors for decoded-time and calendar time structures instead of list accessors. --- lisp/org.el | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index fc51d4ba3..09992fc10 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -15642,10 +15642,11 @@ org-timestamp-change (not current-prefix-arg)) ;; This looks like s-up and s-down. Change by one rounding step. (progn - (setq increment (* dm (cond ((> n 0) 1) ((< n 0) -1) (t 0)))) - (unless (= 0 (setq rem (% (nth 1 time0) dm))) - (setcar (cdr time0) (+ (nth 1 time0) - (if (> n 0) (- rem) (- dm rem)))))) + (setq increment (* dm (cond ((> n 0) 1) ((< n 0) -1) (t 0)))) + (unless (= 0 (setq rem (% (decoded-time-minute time0) dm))) + (setf (decoded-time-minute time0) + (+ (decoded-time-minute time0) + (if (> n 0) (- rem) (- dm rem)))))) ;; Do not round anything in `org-modify-ts-extra' when prefix ;; argument is supplied - just use whatever is provided by the ;; prefix argument. @@ -15677,13 +15678,15 @@ org-timestamp-change (setq extra (org-modify-ts-extra extra timestamp? n dm))) (when (eq what 'calendar) (let ((cal-date (org-get-date-from-calendar))) - (setcar (nthcdr 4 time0) (nth 0 cal-date)) ; month - (setcar (nthcdr 3 time0) (nth 1 cal-date)) ; day - (setcar (nthcdr 5 time0) (nth 2 cal-date)) ; year - (setcar time0 (or (car time0) 0)) - (setcar (nthcdr 1 time0) (or (nth 1 time0) 0)) - (setcar (nthcdr 2 time0) (or (nth 2 time0) 0)) - (setq time (org-encode-time time0)))) + (setq time (org-encode-time + (decoded-time-set-defaults + (make-decoded-time + :month (calendar-extract-month cal-date) + :day (calendar-extract-day cal-date) + :year (calendar-extract-year cal-date) + :second (decoded-time-second time0) + :minute (decoded-time-minute time0) + :hour (decoded-time-hour time0))))))) ;; Insert the new timestamp, and ensure point stays in the same ;; category as before (i.e. not after the last position in that ;; category). base-commit: 7ed519bfa57cbfea637893f5a29c229e356eee40 -- 2.52.0
>From 0584a77f474e6d47d9bcc04bb73479e9dde991e4 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 15 Mar 2026 12:47:59 -0400 Subject: [PATCH 2/7] Add a compatibility shim for the function decoded-time-add * lisp/org-compat.el (org-decoded-time-add): New function to make the fixed version of `decoded-time-add' available. --- lisp/org-compat.el | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lisp/org-compat.el b/lisp/org-compat.el index 0473148b5..55a4e481e 100644 --- a/lisp/org-compat.el +++ b/lisp/org-compat.el @@ -86,6 +86,7 @@ ;; `org-string-equal-ignore-case' is in _this_ file but isn't at the ;; top-level. (declare-function org-string-equal-ignore-case "org-compat" (string1 string2)) +(declare-function decoded-time-add "time-date" (time delta)) (defvar calendar-mode-map) (defvar org-complex-heading-regexp) @@ -111,6 +112,21 @@ org-fold-core-style `(metadata . ,metadata) (complete-with-action action table string pred))))) +;; Broken for negative months before emacs commit bc33b70b280 +(if (version< "31" emacs-version) + (defalias 'org-decoded-time-add #'decoded-time-add) + (defun org-decoded-time-add (time delta) + "See doctring for `decoded-time-add'." + (if (decoded-time-month delta) + (let ((time (copy-sequence time)) + (delta (copy-sequence delta))) + (let ((new (+ (1- (decoded-time-month time)) (decoded-time-month delta)))) + (setf (decoded-time-month time) (1+ (mod new 12))) + (cl-incf (decoded-time-year time) (floor new 12))) + (setf (decoded-time-month delta) nil) + (decoded-time-add time delta)) + (decoded-time-add time delta)))) + ;;; Emacs < 29 compatibility -- 2.52.0
>From f6d98813972384213debbee224adbbe86277d813 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 15 Mar 2026 13:10:46 -0400 Subject: [PATCH 3/7] org-timestamp-change: Fix time addition Previously, trying to add a month to "<2026-01-31 Sat>" resulted in "<2026-03-03 Tue>". Now we get the correct result of "<2026-02-28 Sat>". * lisp/org.el (org-timestamp-change): Use `org-decoded-time-add' to add the time instead of relying of `org-encode-time' to figure it out. * testing/lisp/test-org.el (test-org/org-timestamp-change): Add regression tests. --- lisp/org.el | 18 ++++++++++-------- testing/lisp/test-org.el | 20 +++++++++++++++++++- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index 09992fc10..f09a47569 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -15653,14 +15653,16 @@ org-timestamp-change (setq dm 1)) (setq time (org-encode-time - (apply #'list - (or (car time0) 0) - (+ (if (eq timestamp? 'minute) increment 0) (nth 1 time0)) - (+ (if (eq timestamp? 'hour) increment 0) (nth 2 time0)) - (+ (if (eq timestamp? 'day) increment 0) (nth 3 time0)) - (+ (if (eq timestamp? 'month) increment 0) (nth 4 time0)) - (+ (if (eq timestamp? 'year) increment 0) (nth 5 time0)) - (nthcdr 6 time0))))) + (org-decoded-time-add + time0 + (make-decoded-time + (cl-ecase timestamp? + (minute :minute) + (hour :hour) + (day :day) + (month :month) + (year :year)) + increment))))) (when (and (memq timestamp? '(hour minute)) extra (string-match "-\\([012][0-9]\\):\\([0-5][0-9]\\)" extra)) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index b31e0e5c1..ab3954510 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -9498,7 +9498,25 @@ test-org/org-timestamp-change (should (string= (buffer-substring (point-min) (point-max)) now-ts)) - (forward-char 1)))))))) + (forward-char 1))))))) + ;; Corner cases + (let ((org-timestamp-formats + (cons (replace-regexp-in-string + " %a" "" (car org-timestamp-formats)) + (replace-regexp-in-string + " %a" "" (cdr org-timestamp-formats))))) + (cl-flet ((test-org-timestamp-change (text n what expected) + (org-test-with-temp-text text + (org-timestamp-change n what) + (should (equal expected (buffer-string)))))) + ;; Changing between months with different number of days + (test-org-timestamp-change "<2026-01-31>" 1 'month "<2026-02-28>") + (test-org-timestamp-change "<2026-02-28>" 1 'month "<2026-03-28>") + (test-org-timestamp-change "<2026-03-31>" -1 'month "<2026-02-28>") + ;; Year boundry + (test-org-timestamp-change "<2025-12-31>" 1 'month "<2026-01-31>") + (test-org-timestamp-change "<2025-12-31>" 2 'month "<2026-02-28>") + (test-org-timestamp-change "<2026-02-28>" -2 'month "<2025-12-28>")))) (ert-deftest test-org/timestamp () "Test `org-timestamp' specifications." -- 2.52.0
>From 9ff322f9354a92abe170f5cfaaa779bbce10e635 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Fri, 5 Dec 2025 18:50:31 -0500 Subject: [PATCH 4/7] test-org/org-timestamp-change: Relax test requirements This had been fixed previously in commit c81c844d1 by hard coding the date. Relaxing the requirements allows for more testing. * testing/lisp/test-org.el (test-org/org-timestamp-change): Run the test using at most the 28th day of the month. --- testing/lisp/test-org.el | 79 +++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index ab3954510..7bdb0c5c9 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -9462,43 +9462,48 @@ test-org/at-timestamp-p (ert-deftest test-org/org-timestamp-change () "Test `org-timestamp-change' specifications." - (org-test-at-time "2026-01-15" - (let ((now (current-time)) now-ts point) - (message "Testing with timestamps <%s> and <%s>" - (format-time-string (car org-timestamp-formats) now) - (format-time-string (cdr org-timestamp-formats) now)) - ;; loop over regular timestamp formats and weekday-less timestamp - ;; formats - (dolist (org-timestamp-formats - (list org-timestamp-formats - (cons (replace-regexp-in-string - " %a" "" (car org-timestamp-formats)) - (replace-regexp-in-string - " %a" "" (cdr org-timestamp-formats))))) - ;; loop over timestamps that do not and do contain time - (dolist (format (list (car org-timestamp-formats) - (cdr org-timestamp-formats))) - (setq now-ts - (concat "<" (format-time-string format now) ">")) - (org-test-with-temp-text now-ts - (forward-char 1) - (while (not (eq (char-after) ?>)) - (skip-syntax-forward "-") - ;; change the timestamp unit at point one down, two up, - ;; one down, which should give us the original timestamp - ;; again. However, point can move backward during that - ;; operation, so take care of that. *Not* using - ;; `save-excursion', which fails to restore point since - ;; the timestamp gets completely replaced. - (setq point (point)) - (org-timestamp-change -1 nil nil nil) - (org-timestamp-change 2 nil nil nil) - (org-timestamp-change -1 nil nil nil) - (goto-char point) - (should (string= - (buffer-substring (point-min) (point-max)) - now-ts)) - (forward-char 1))))))) + (let ((now (decode-time)) now-ts point) + ;; Decrementing a month from March 31st yields February + ;; 28th. This particular test is easier to write if the + ;; days don't change when modifying the month + (setf (decoded-time-day now) + (min (decoded-time-day now) 28)) + (setq now (encode-time now)) + (message "Testing with timestamps <%s> and <%s>" + (format-time-string (car org-timestamp-formats) now) + (format-time-string (cdr org-timestamp-formats) now)) + ;; loop over regular timestamp formats and weekday-less timestamp + ;; formats + (dolist (org-timestamp-formats + (list org-timestamp-formats + (cons (replace-regexp-in-string + " %a" "" (car org-timestamp-formats)) + (replace-regexp-in-string + " %a" "" (cdr org-timestamp-formats))))) + ;; loop over timestamps that do not and do contain time + (dolist (format (list (car org-timestamp-formats) + (cdr org-timestamp-formats))) + (setq now-ts + (concat "<" (format-time-string format now) ">")) + (org-test-with-temp-text now-ts + (forward-char 1) + (while (not (eq (char-after) ?>)) + (skip-syntax-forward "-") + ;; change the timestamp unit at point one down, two up, + ;; one down, which should give us the original timestamp + ;; again. However, point can move backward during that + ;; operation, so take care of that. *Not* using + ;; `save-excursion', which fails to restore point since + ;; the timestamp gets completely replaced. + (setq point (point)) + (org-timestamp-change -1 nil nil nil) + (org-timestamp-change 2 nil nil nil) + (org-timestamp-change -1 nil nil nil) + (goto-char point) + (should (string= + (buffer-substring (point-min) (point-max)) + now-ts)) + (forward-char 1)))))) ;; Corner cases (let ((org-timestamp-formats (cons (replace-regexp-in-string -- 2.52.0
>From 576f42d73d9afd45d2d87076bbdbb1a921e6038f Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 15 Mar 2026 13:32:48 -0400 Subject: [PATCH 5/7] test-org/org-timestamp-change: Test minute rounding * testing/lisp/test-org.el (test-org/org-timestamp-change): Add tests for rounding the minutes according to `org-time-stamp-rounding-minutes'. --- testing/lisp/test-org.el | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 7bdb0c5c9..82beb9d58 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -9521,7 +9521,20 @@ test-org/org-timestamp-change ;; Year boundry (test-org-timestamp-change "<2025-12-31>" 1 'month "<2026-01-31>") (test-org-timestamp-change "<2025-12-31>" 2 'month "<2026-02-28>") - (test-org-timestamp-change "<2026-02-28>" -2 'month "<2025-12-28>")))) + (test-org-timestamp-change "<2026-02-28>" -2 'month "<2025-12-28>"))) + ;; Rounding from `org-time-stamp-rounding-minutes' + (cl-flet ((test-time-stamp-rounding (text rounding amount expected) + (let ((org-time-stamp-rounding-minutes (list 0 rounding))) + (org-test-with-temp-text text + (org-timestamp-change amount 'minute 'updown) + (re-search-forward org-ts-regexp1) + (should (equal expected (string-trim (match-string 6)))))))) + (dotimes (i 9) + (test-time-stamp-rounding "<2026-03-14 12:00>" (1+ i) 1 + (concat "12:0" (number-to-string (1+ i))))) + (dotimes (i 9) + (test-time-stamp-rounding "<2026-03-14 12:00>" (1+ i) -1 + (concat "11:5" (number-to-string (- 9 i))))))) (ert-deftest test-org/timestamp () "Test `org-timestamp' specifications." -- 2.52.0
>From 37980b666974873754bbcc259a12b427b6d81851 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sat, 14 Mar 2026 16:15:19 -0400 Subject: [PATCH 6/7] org-clock-timestamps-change: check location in timestamp Previously running this function with point on the brackets of a timestamp or with the point just after a timestamp would result in a puzzling error. Now the error shouldn't occur, and if it does it will be more descriptive. * lisp/org-clock.el (org-clock-timestamps-change): Guard against certain timestamp locations and try not to divide by zero. --- lisp/org-clock.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/org-clock.el b/lisp/org-clock.el index ce2d23a9b..20dd1e88f 100644 --- a/lisp/org-clock.el +++ b/lisp/org-clock.el @@ -1936,7 +1936,7 @@ org-clock-timestamps-change 'org-timestamp-down)) (timestamp? (org-at-timestamp-p 'lax)) ts1 begts1 ts2 begts2 updatets1 tdiff) - (when timestamp? + (when (not (memq timestamp? '(nil bracket after))) (save-excursion (move-beginning-of-line 1) (re-search-forward org-ts-regexp3 nil t) @@ -1967,7 +1967,7 @@ org-clock-timestamps-change (goto-char begts) (org-timestamp-change (round (/ (float-time tdiff) - (pcase timestamp? + (pcase-exhaustive timestamp? (`minute 60) (`hour 3600) (`day (* 24 3600)) -- 2.52.0
>From d35a1b3b74a5a5fdab0a4f7154284903fda174d3 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sat, 14 Mar 2026 16:21:05 -0400 Subject: [PATCH 7/7] test-org-clock/org-clock-timestamps-change: Add more test coverage * testing/lisp/test-org-clock.el (test-org-clock/org-clock-timestamps-change): Add more test coverage inspired by the test `test-org/org-timestamp-change'. --- testing/lisp/test-org-clock.el | 45 +++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el index 034446548..fc1fa15c9 100644 --- a/testing/lisp/test-org-clock.el +++ b/testing/lisp/test-org-clock.el @@ -120,7 +120,50 @@ test-org-clock/org-clock-timestamps-change (org-test-with-temp-text "CLOCK: [2023-02-19 Sun 2<point>3:30]--[2023-02-20 Mon 00:35] => 1:05" (org-clock-timestamps-change 'up 1) - (buffer-string)))))) + (buffer-string))))) + (let ((org-time-stamp-rounding-minutes '(1 1)) ;; No rounding! + (now (decode-time)) test-text point) + ;; Decrementing a month from March 31st yields February + ;; 28th. This particular test is easier to write if the + ;; days don't change when modifying the month. + (setf (decoded-time-day now) + (min (decoded-time-day now) 28)) + (setq now (encode-time now)) + ;; loop over regular timestamp formats and weekday-less timestamp + ;; formats + (dolist (org-timestamp-formats + (list org-timestamp-formats + (cons (replace-regexp-in-string + " %a" "" (car org-timestamp-formats)) + (replace-regexp-in-string + " %a" "" (cdr org-timestamp-formats))))) + (setq test-text + (concat "CLOCK: [" + (format-time-string (cdr org-timestamp-formats) now) + "]--[" + (format-time-string (cdr org-timestamp-formats) (time-add now (* 60 60))) + "] => 1:00")) + (org-test-with-temp-text test-text + (while (not (eolp)) + ;; change the timestamp unit at point one down, two up, + ;; one down, which should give us the original timestamp + ;; again. However, point can move backward during that + ;; operation, so take care of that. *Not* using + ;; `save-excursion', which fails to restore point since + ;; the timestamp gets completely replaced. + (setq point (point)) + (org-clock-timestamps-down) + (let ((current-prefix-arg '(2))) + ;; We supply the prefix argument 2 to increment the + ;; minutes by 2. We supply the function argument 2 for + ;; everything else. Is this a bug? Probably. FIXME. + (org-clock-timestamps-up 2)) + (org-clock-timestamps-down) + (goto-char point) + (should (string= + (buffer-substring (point-min) (point-max)) + test-text)) + (forward-char 1)))))) (ert-deftest test-org-clock/org-clock-update-time-maybe () "Test `org-clock-update-time-maybe' specifications." -- 2.52.0
