bug#24778: 25.1; cl-assert always calls the default debugger on assertion failures

[npostavs]

tags 24778 confirmed
quit

David Shepherd <davidshepherd7@gmail.com> writes:

> This causes problems with ert if you use cl-assert.
>

Yes, I've hit this too, I had to advise cl--assertion-failed to get
around it.

  (defadvice cl--assertion-failed (around el-get-test-suppress-debugger 
activate)
    "Prevent failed `assert' from jumping into debugger."
    (let ((debug-on-error nil))
      ad-do-it))

[...]

>
> Older versions of emacs gave something like the second output even when
> cl-assert was used.
>
>
> This doesn't seem to be an issue if ert tests are run interactively, I'm
> not sure why.

Interactively, if you continue from the debugger, the test passes, which
I would also consider wrong (since an assertion failed).

>
>
> Removing the lines
>
>     (if debug-on-error
>        (apply debugger `(cl-assertion-failed ,form ,string ,@sargs))
>
> from cl--assertion-failed seems to solve the problem, but I don't know
> if that would break other things.

This debug-on-error clause is new in 25.1 (specifically, it was added by
61b4c22c "* lisp/emacs-lisp/cl*.el: Use define-inline and move some
code..."), so this looks like a regression to me (and it looks
accidental, at least the commit message says nothing about it).  I'd
like to remove it for 25.2.  Any objections?