bug#50629: 28.0.50; hard to debug an uncaught error with ert

["Gerd Möllmann" <gerd.moellmann@gmail.com>]

Ihor Radchenko <yantar92@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> On 22-08-27 16:29 , Lars Ingebrigtsen wrote:
>>> 			  (args-574 (condition-case err (let ( foo)
>>> 							  (list 42 (foo)))
>>
>> Sorry, you are right, but then the condition-case "swallows" the error.
>> Not sure what can be done about that.
>
> condition-case-unless-debug?

Well, that's a nice fretwork :-)

When I expand the er-deftest form I get something like this

(progn
  (ert-set-test 'f
		(record 'ert-test 'f nil
			#'(lambda nil
			    (setq debug-on-error t)
			    (defalias 'foo
			      #'(lambda nil
				  (bar)))
			    (defalias 'bar
			      #'(lambda nil
				  (zot)))
			    (let*
				((fn-339 #'equal)
				 (args-340
				  (condition-case err
				      (let
					  ((signal-hook-function #'ert--should-signal-hook))
					(list 42
					      (foo)))
				    (error
				     (progn
				       (setq fn-339 #'signal)
				       (list
					(car err)
					(cdr err)))))))
...

Note the binding of signal-hook-function, which is missing from Lars'
expansion result, where is says

		   (let* ((fn-573 #'equal)
			  (args-574 (condition-case err (let ( foo)
							  (list 42 (foo)))
				      (error (progn
					       (setq fn-573 #'signal)
					       (list (car err) (cdr
			  err)))))))

I can't explain that.

Anyway, let's assume that signal-hook-function is set. and the test is
run.  This is done in ert--run-test-internal.  That function binds
debug-on-error to t, and debugger to a lambda invoking
ert--run-test-debugger.

When our error is signaled, ert--should-signal-hook does nothing because
the error is not one of (ert-test-failed ert-test-skipped).  So
ert--run-test-debugger is not run during the original signal.  Not from
the signal-hook-function, because it doesn't want to, and not otherwise
because of the condition-case.

Instead, this part of the test expansion comes into play:

			      (let
				  ((value-341 'ert-form-evaluation-aborted-342))
				(let
				    (form-description-343)
				  (if
				      (unwind-protect
					  (setq value-341
						(apply fn-339 args-340))

Note that fn-339 has been set to signal in the condition-case.  Since
debug-on-error is t, the apply in the last line calls signal and we land
in ert--run-test-debugger.  The debugger than records the backtrace,
which is of course not the original backtrace.

So, what can we do?

I assume there is a good reason that the signal hook function doesn't
invoke the debugger?  Or in other words, what breaks when the debugger
is always invoked, which it would with condition-case-unless-error?
(That would be when ert-fail or ert-skip are used in tests.)

I don't know.

Interestingly with condition-case-unless I get one less error with make
check:

With condition-case-unless-debug:
SUMMARY OF TEST RESULTS
-----------------------
Files examined: 445
Ran 6582 tests, 6410 results as expected, 7 unexpected, 165 skipped
5 files contained unexpected results:
  lisp/subr-tests.log
  lisp/ls-lisp-tests.log
  lisp/emacs-lisp/ert-tests.log
  lisp/emacs-lisp/edebug-tests.log

With condition-case:
SUMMARY OF TEST RESULTS
-----------------------
Files examined: 445
Ran 6582 tests, 6409 results as expected, 8 unexpected, 165 skipped
6 files contained unexpected results:
  src/process-tests.log
  lisp/subr-tests.log
  lisp/ls-lisp-tests.log
  lisp/emacs-lisp/ert-tests.log
  lisp/emacs-lisp/edebug-tests.log
  lisp/emacs-lisp/cl-lib-tests.log

Could others try this on their platforms/with their test suites?

From 8154955c5ff056d60b6bcd0b4b2dd234c33f0a75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <gerd@gnu.org>
Date: Sun, 28 Aug 2022 11:23:00 +0200
Subject: [PATCH] Let ERT test definitions use condition-case-unless-debug

* lisp/emacs-lisp/ert.el (ert--expand-should-1): Use
condition-case-unless-debug in test expansion (bug#50629).
---
 lisp/emacs-lisp/ert.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 047b0069bb..a8fe14aa6d 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -319,7 +319,9 @@ ert--expand-should-1
               (value (gensym "value-"))
               (default-value (gensym "ert-form-evaluation-aborted-")))
           `(let* ((,fn (function ,fn-name))
-                  (,args (condition-case err
+                  ;; Use condition-case-unless-debug to let the debugger record
+                  ;; the original backtrace when a signal occurs.
+                  (,args (condition-case-unless-debug err
                              (let ((signal-hook-function #'ert--should-signal-hook))
                                (list ,@arg-forms))
                            (error (progn (setq ,fn #'signal)
-- 
2.37.2