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

[Gerd Möllmann]

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