`assert` does not work on emacs 24.4+ and ecukes

[Wilfred]

On Emacs 24.4 and later, assert (which is cl-assert) directly calls the debug function on failure:

(defun cl--assertion-failed (form &optional string sargs args)
  (if debug-on-error
      (debug `(cl-assertion-failed ,form ,string ,@sargs))
    (if string
        (apply #'error string (append sargs args))
      (signal 'cl-assertion-failed `(,form ,@sargs)))))

This leads to epic backtraces with ecukes: https://gist.github.com/Wilfred/e86988f0b1a51eaf3ca6cec01fcbb7e3 and the test run terminating, instead of the usual friendly test failure message with passed/failed totals.

This can be fixed by removing the following line from ecukes-cli.el:

(setq debug-on-error t)

Alternatively, we could redefine the debug function so that it uses ecukes-debug. What do you think?

[rejeep]

See https://github.com/ecukes/espuds/pull/41

[davidshepherd7]

@Wilfred I'm working around this problem for now with this patch. Just drop that code into something that is loaded when your tests run and you'll get sane stack traces again.

This is a bug in emacs 25.1 itself, it should be fixed in 25.2 (but currently there might be other issues with using ecukes in the pretest version).

[neojski]

I think it's still failing in 25.2 and that I'm experiencing this in https://github.com/magnars/expand-region.el/pull/231.

[BitPuffin]

Yep, still got this is 25.2.1 as well. The code for assertion failed is currently:

(defun cl--assertion-failed (form &optional string sargs args)
  (if debug-on-error
      (funcall debugger 'error `(cl-assertion-failed (,form ,string ,@sargs)))
    (if string
        (apply #'error string (append sargs args))
      (signal 'cl-assertion-failed `(,form ,@sargs)))))

So it's still calling debug directly (since the value of debugger is 'debug).

So the good news is that ecukes could probably change the value of debugger and get this working?

[davidshepherd7]

From the emacs log it looks like the patch in 25.2 which fixed this bug was partially reverted:

commit eb364fddec1431f459166cebb36f09f6b371dd71
Author: Noam Postavsky <npostavs@gmail.com>
Date:   Mon Nov 7 20:03:48 2016 -0500

    Do call debugger on failed cl-assert

    "Don't call debug on failed cl-assert..." removed the call to `debug' in
    cl--assertion-failed because `debug' calls `kill-emacs' in batch mode,
    thus messing up ert test runs.  However, calling the debugger is useful
    because it allows catching failed assertions even inside
    `condition-case' calls.  The problem with ert can be avoided by calling
    `debugger' instead of `debug' directly, since ert installs its own
    debugger while running tests.

    * lisp/emacs-lisp/cl-preloaded.el (cl--assertion-failed): Call
    `debugger' if `debug-on-error' is non-nil.

diff --git a/lisp/emacs-lisp/cl-preloaded.el b/lisp/emacs-lisp/cl-preloaded.el
index 639ffa6..2b022c4 100644
--- a/lisp/emacs-lisp/cl-preloaded.el
+++ b/lisp/emacs-lisp/cl-preloaded.el
@@ -44,9 +44,11 @@
 (define-error 'cl-assertion-failed (purecopy "Assertion failed"))

 (defun cl--assertion-failed (form &optional string sargs args)
-  (if string
-      (apply #'error string (append sargs args))
-    (signal 'cl-assertion-failed `(,form ,@sargs))))
+  (if debug-on-error
+      (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs))
+    (if string
+        (apply #'error string (append sargs args))
+      (signal 'cl-assertion-failed `(,form ,@sargs)))))

 ;; When we load this (compiled) file during pre-loading, the cl--struct-class
 ;; code below will need to access the `cl-struct' info, since it's considered

@BitPuffin Unfortunately ecukes uses a custom debugger to extract a backtrace, so we can't just set debug-on-error to nil.

After some investigation: the problem is that ecukes uses condition-case to handle errors and a custom debugger just to capture backtraces. However the current implementation of cl-assert directly calls the debugger without throwing an error, so now we capture the backtrace but never record the error. In other words all tests appear to pass even when they fail!

It looks like the current implementation is a convenient hack to allow easier debugging of assertion failures even when the error is handled further up the stack. There's talk of a proper fix in emacs 26, but that could be years away.

I've added a note about this to the readme.

[npostavs]

Unfortunately ecukes uses a custom debugger to extract a backtrace, so we can't just set debug-on-error to nil.

After some investigation: the problem is that ecukes uses condition-case to handle errors and a custom debugger just to capture backtraces. However the current implementation of cl-assert directly calls the debugger without throwing an error, so now we capture the backtrace but never record the error. In other words all tests appear to pass even when they fail!

Can you change the custom debugger function to record an error when it gets error as the first argument? ert also uses a custom debugger to save the backtrace, that's how it works with the (funcall debugger ...) code in 25.2, though not the (debug ...) in 25.1.

[BitPuffin]

@davidshepherd7 I think you misunderstood me. I was not saying that you set debug-on-error to nil. Rather, I was suggesting that you change the value of debugger to one that ecukes can use to extract error information.

The relevant part of the excerpt from cl--assertion-failed being:

(funcall debugger 'error `(cl-assertion-failed (,form ,string ,@sargs)))

Evaluating debugger in ielm shows us that it's simply the debug function

ELISP> debugger
debug
ELISP> (functionp debugger)
t

More info can be found with C-h v debugger

So would it help to set the debugger variable to the ecukes custom function or not?

[davidshepherd7]

...ert also uses a custom debugger to save the backtrace...

Yeah good idea I'll have a look at how ert does it. I wasn't too sure about raising a new error from the custom debugger because I was worried it might result in the same error being raised twice (for non cl-assert errors).

@BitPuffin Oh sorry I misread your comment, it should already be using a custom debug function which is set here. We might be able to fix the problem by modifying what that custom function does though.

[rcj]

@davidshepherd7 This seems to also affect emacs 26

[davidshepherd7]

@rcj Thanks, I'll apply the changes in emacs 26 as well. I guess someone needs to pester some emacs core devs about fixing this properly...