From 01ad14ab75d5d64aadd15af8fc0d1dd61e2ce4bd Mon Sep 17 00:00:00 2001
From: Craig Jennings <c@cjennings.net>
Date: Tue, 5 May 2026 10:55:03 -0500
Subject: test: shadow signal-hook-function so cl-assertion-failed can be
 caught
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Attempt 4 at making the eight scheduler error tests pass on
Emacs 29.4 in CI.

The earlier approaches kept failing because ERT 29.4 installs
ert--should-signal-hook as signal-hook-function around the entire
ert-deftest body — not just inside should forms.  That hook fires
on every signal before any inner condition-case can catch it,
which is why even a bare (condition-case ... (cl-assertion-failed
nil)) at the top of the test body didn't work.

The new helper rebinds signal-hook-function to nil inside its own
let-scope, so condition-case catches the cl-assertion-failed
signal normally.  The ert-fail on the no-error path runs outside
that shadowing scope, so it still routes through ERT's failure
handling.

Locally green; pushing to test 29.4 in CI.
---
 ...st-org-drill-determine-next-interval-simple8.el | 32 ++++++++++++++--------
 .../test-org-drill-determine-next-interval-sm5.el  | 13 +++++----
 2 files changed, 28 insertions(+), 17 deletions(-)

(limited to 'tests')

diff --git a/tests/test-org-drill-determine-next-interval-simple8.el b/tests/test-org-drill-determine-next-interval-simple8.el
index 667894a..ed90285 100644
--- a/tests/test-org-drill-determine-next-interval-simple8.el
+++ b/tests/test-org-drill-determine-next-interval-simple8.el
@@ -208,20 +208,28 @@ review attempt regardless of which scheduling algorithm produced it."
 (defmacro test-scheduler--should-cl-assert (&rest body)
   "Assert BODY signals a cl-assertion-failed via condition-case.
 
-Avoids `should-error' (and `should' generally) because ERT in Emacs
-29.4 installs `signal-hook-function' around `should' forms — that
-hook intercepts signals before any inner condition-case can catch
-them, so the plain (should-error ...) wrap fails on 29.4 even
-though the cl-assertion-failed signal does fire.
+ERT 29.4 installs `ert--should-signal-hook' as `signal-hook-function'
+around the entire `ert-deftest' body — not just `should' forms.  That
+hook intercepts every signal before any inner `condition-case' can
+catch it, so the obvious (should-error FORM) and even a manual
+condition-case both fail on 29.4 even though cl-assertion-failed
+clearly fires (visible in the test-failure backtrace).
+
+The fix is to shadow `signal-hook-function' to nil inside our
+wrapper, letting condition-case catch normally.  The ert-fail call
+on the no-error path runs after our shadowing scope exits, so it
+still routes through ERT's normal failure handling.
 
 Catches `cl-assertion-failed' by name rather than via the generic
-`error' parent: the parent inheritance for cl-assertion-failed is
-inconsistent across Emacs versions, but the symbol-name match
-through condition-case always works."
-  `(condition-case _err
-       (progn ,@body
-              (ert-fail "expected cl-assertion-failed signal, got none"))
-     (cl-assertion-failed nil)))
+`error' parent — inheritance varies across Emacs versions but the
+symbol-name match through condition-case always works."
+  `(let ((caught
+          (let ((signal-hook-function nil))
+            (condition-case _err
+                (progn ,@body 'no-error)
+              (cl-assertion-failed 'caught)))))
+     (unless (eq caught 'caught)
+       (ert-fail "expected cl-assertion-failed signal, got none"))))
 
 (ert-deftest test-org-drill-determine-next-interval-simple8-error-negative-repeats ()
   "Error: repeats=-1 violates the (cl-assert (>= repeats 0)) precondition."
diff --git a/tests/test-org-drill-determine-next-interval-sm5.el b/tests/test-org-drill-determine-next-interval-sm5.el
index e0ac464..de476a4 100644
--- a/tests/test-org-drill-determine-next-interval-sm5.el
+++ b/tests/test-org-drill-determine-next-interval-sm5.el
@@ -248,11 +248,14 @@ The SM5 floor is shared with SM2 via `org-drill-modify-e-factor'.")
   "Assert BODY signals a cl-assertion-failed via condition-case.
 
 Mirrors the simple8 test file's helper.  See its commentary for why
-this avoids `should-error' / `should' on Emacs 29.4."
-  `(condition-case _err
-       (progn ,@body
-              (ert-fail "expected cl-assertion-failed signal, got none"))
-     (cl-assertion-failed nil)))
+shadowing `signal-hook-function' is necessary on Emacs 29.4."
+  `(let ((caught
+          (let ((signal-hook-function nil))
+            (condition-case _err
+                (progn ,@body 'no-error)
+              (cl-assertion-failed 'caught)))))
+     (unless (eq caught 'caught)
+       (ert-fail "expected cl-assertion-failed signal, got none"))))
 
 (ert-deftest test-org-drill-determine-next-interval-sm5-error-negative-n ()
   "Error: n=-1 violates the (cl-assert (> n 0)) precondition."
-- 
cgit v1.2.3