Subject: bug#47320: Improve failure reporting in test/lisp/electrict-tests.el

From: João Távora <[email-masked]>
Date: Wed, 24 Mar 2021 20:10:48 +0000

Hello Alan,

> I just committed the patch as it was.  Sorry.

I don't much see the point in asking for comments from people
and then proceeding to ignore the simplest request for adjustments
outright.
You did write "if there are no objections...". Well I had a small
objection. I get it that following Michael's idea, which is probably
better, takes a little bit more work, but a simple adjustment
to avoid code repetition could certainly be accommodated.

FWIW I've quickly tried with this version and it seems to work
fine:

(defun electric-pair-test-for (fixture where char expected-string
                                       expected-point mode bindings
                                       fixture-fn &optional doc-string)
  (with-temp-buffer
    (funcall mode)
    (insert fixture)
    (save-electric-modes
      (let ((last-command-event char)
            (transient-mark-mode 'lambda))
        (goto-char where)
        (funcall fixture-fn)
        (cl-progv
            (mapcar #'car bindings)
            (mapcar #'cdr bindings)
          (call-interactively (key-binding `[,last-command-event])))))
    (unless (equal (buffer-substring-no-properties (point-min) (point-max))
                   expected-string)
      (when doc-string (message "\n%s\n" doc-string))
      (ert-fail (format
                 "buffer contents don't match! (observed %s, expected %s)"
                 (buffer-string) expected-string)))
    (unless (equal (point) expected-point)
      (when doc-string (message "\n%s\n" doc-string))
      (ert-fail
       (format "point isn't where it was supposed to be! (observed %s,
expected %s)"
               (point) expected-point)))))

On Wed, Mar 24, 2021 at 7:42 PM Alan Mackenzie <[email-masked]> wrote:
>
> Hello again, João.
>
> On Wed, Mar 24, 2021 at 14:02:58 +0000, João Távora wrote:
> > On Wed, Mar 24, 2021 at 1:46 PM Alan Mackenzie <[email-masked]> wrote:
>
> > > I'm having some difficulty getting my head around the "explanation"
> > > functionality of ert, which leans me towards my initial plan.
>
> > > Here's the patch (actually written quite a long time ago) I would
like to
> > > merge into electric-tests.el:
>
> > As far as I understand, this just stores the big explanation string in
> > a variable, and uses it both for putting into the `ert-deftest` 's
docstring
> > as well as for logging with message(), right?  If so, it's fine to add.
>
> > Maybe you could leverage `ert-fail` instead of checking the test's
> > main assertion twice, once in should, and once when deciding
> > whether to log.
>
> In the end, I couldn't get that to work - the handler for the signal, in
> outputting the doc string, replaced all the \n's with the octal
> read-syntax, "\\12".  This left the text readable only with effort.
>
> > I'd say, just check it once and put both logging and `ert-fail` inside
the if.
>
> I just committed the patch as it was.  Sorry.
>
> > João
>
> --
> Alan Mackenzie (Nuremberg, Germany).

-- 
João Távora