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

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 24 Mar 2021 19:44:02 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
47320: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=47320
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#47320: Improve failure reporting in test/lisp/electrict-tests.el
From: Alan Mackenzie <[email-masked]>
To: João Távora <[email-masked]>
Cc: Michael Albinus <[email-masked]>, [email-masked]
Date: Wed, 24 Mar 2021 19:42:54 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

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).

Subject: Improve failure reporting in test/lisp/electrict-tests.el
From: Alan Mackenzie <[email-masked]>
To: [email-masked]
Cc: João Távora <[email-masked]>
Date: Mon, 22 Mar 2021 14:24:37 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello, Emacs.

When running make check, and a failure occurs in electric-tests.elc, the
resulting output in electric-tests.log is frustratingly cryptic.
Although one can fairly easily get back to the source code for the test
in electric-tests.el, it is difficult to reconstruct the snippet of text
on which the test was performed.  This is because the tests are
(necessarily) generated by fairly inscrutable macros.

This is a shame, since each generated test has its own exceptionally
clear generated doc-string.  An example of such a doc string is:

#########################################################################
Electricity test in a `c-mode' buffer.

Start with point at 7 in a 7-char-long buffer
like this one:

  |"foo \"|   (buffer start and end are denoted by `|')

Now call this:

#'electric-quote-local-mode

Ensure the following bindings:

'((electric-quote-replace-double . t)
  (electric-quote-comment . t)
  (electric-quote-string . t))

Now press the key for: "

The buffer's contents should become:

  |"foo \""|

, and point should be at 7.
#########################################################################

My proposal is that on a test failure, this generated doc-string should
be output along with the other failure stuff.  It doesn't actually add
all that much bulk to the .log file.

Here is patch which does this.  If there are no objections, I will
commit it in a day or two.

diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 62a42b7fe4..44b3d8b672 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -50,7 +50,8 @@ save-electric-modes
   `(call-with-saved-electric-modes #'(lambda () ,@body)))
 
 (defun electric-pair-test-for (fixture where char expected-string
-                                       expected-point mode bindings fixture-fn)
+                                       expected-point mode bindings
+                                       fixture-fn &optional doc-string)
   (with-temp-buffer
     (funcall mode)
     (insert fixture)
@@ -63,6 +64,14 @@ electric-pair-test-for
             (mapcar #'car bindings)
             (mapcar #'cdr bindings)
           (call-interactively (key-binding `[,last-command-event])))))
+    (when
+        (and doc-string
+             (not
+              (and
+               (equal (buffer-substring-no-properties (point-min) (point-max))
+                      expected-string)
+               (equal (point) expected-point))))
+      (message "\n%s\n" doc-string))
     (should (equal (buffer-substring-no-properties (point-min) (point-max))
                    expected-string))
     (should (equal (point)
@@ -109,14 +118,9 @@ electric-pair-test-for
            (fixture (format "%s%s%s" prefix fixture suffix))
            (expected-string (format "%s%s%s" prefix expected-string suffix))
            (expected-point (+ (length prefix) expected-point))
-           (pos (+ (length prefix) pos)))
-      `(ert-deftest ,(intern (format "electric-pair-%s-at-point-%s-in-%s%s"
-                                     name
-                                     (1+ pos)
-                                     mode
-                                     extra-desc))
-           ()
-         ,(format "Electricity test in a `%s' buffer.\n
+           (pos (+ (length prefix) pos))
+           (doc-string
+            (format "Electricity test in a `%s' buffer.\n
 Start with point at %d in a %d-char-long buffer
 like this one:
 
@@ -143,7 +147,14 @@ electric-pair-test-for
                   char
                   (if (string= fixture expected-string) "stay" "become")
                   (replace-regexp-in-string "\n" "\\\\n" expected-string)
-                  expected-point)
+                  expected-point)))
+      `(ert-deftest ,(intern (format "electric-pair-%s-at-point-%s-in-%s%s"
+                                     name
+                                     (1+ pos)
+                                     mode
+                                     extra-desc))
+           ()
+         ,doc-string
          (electric-pair-test-for ,fixture
                                  ,(1+ pos)
                                  ,char
@@ -151,7 +162,8 @@ electric-pair-test-for
                                  ,expected-point
                                  ',mode
                                  ,bindings
-                                 ,fixture-fn)))))
+                                 ,fixture-fn
+                                 ,doc-string)))))
 
 (cl-defmacro define-electric-pair-test
     (name fixture

-- 
Alan Mackenzie (Nuremberg, Germany).