bug#47071: 28.0.50; Show ERT failure reason in batch mode

bug#47071: 28.0.50; Show ERT failure reason in batch mode

[Julian Scheid via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Severity: normal
Tags: patch

`ert-fail' and `ert-skip' both require an argument described as "[data
that] is displayed to the user and should state the reason of the
failure [or the reason for skipping]."

The data is only displayed in interactive mode as a side-effect
however, not in batch mode.  I think it's useful to see the failure or
skip reason in batch mode as well, which is why I would like to
suggest the following change.

(I've borrowed `print-level' etc. from other parts of ERT code but
don't have a strong opinion on it.  I'd imagine that most of the time
people would just pass in a short string anyway.  That's what a quick
grep of Emacs code seems to point to as well.)


From 053abfe6e9a35a76b7f0af866a6336fb35d959f9 Mon Sep 17 00:00:00 2001
From: Julian Scheid <jscheid@protonmail.com>
Date: Thu, 11 Mar 2021 22:49:11 +1300
Subject: [PATCH] Output test failure or skip reason in batch mode

* lisp/emacs-lisp/ert.el (ert-reason-for-test-result): New function.
(ert-run-tests-batch): Output failure or skip reason.
---
 lisp/emacs-lisp/ert.el | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index e91ec0af44..05edbb6fd1 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -1279,6 +1279,23 @@ ert-string-for-test-result
              (ert-test-quit '("quit" "QUIT")))))
     (elt s (if expectedp 0 1))))

+(defun ert-reason-for-test-result (result)
+  "Return the reason given for RESULT, as a string.
+
+The reason is the argument given when invoking `ert-fail' or `ert-skip'.
+It is output using `prin1' prefixed by two spaces.
+
+If no reason was given, or for a successful RESULT, return the
+empty string."
+  (let ((reason
+         (and
+          (ert-test-result-with-condition-p result)
+          (cadr (ert-test-result-with-condition-condition result))))
+        (print-escape-newlines t)
+        (print-level 6)
+        (print-length 10))
+    (if reason (format "  %S" reason) "")))
+
 (defun ert--pp-with-indentation-and-newline (object)
   "Pretty-print OBJECT, indenting it to the current column of point.
 Ensures a final newline is inserted."
@@ -1369,18 +1386,20 @@ ert-run-tests-batch
               (cl-loop for test across (ert--stats-tests stats)
                        for result = (ert-test-most-recent-result test) do
                        (when (not (ert-test-result-expected-p test result))
-                         (message "%9s  %S"
+                         (message "%9s  %S%s"
                                   (ert-string-for-test-result result nil)
-                                  (ert-test-name test))))
+                                  (ert-test-name test)
+                                  (ert-reason-for-test-result result))))
               (message "%s" ""))
             (unless (zerop skipped)
               (message "%s skipped results:" skipped)
               (cl-loop for test across (ert--stats-tests stats)
                        for result = (ert-test-most-recent-result test) do
                        (when (ert-test-result-type-p result :skipped)
-                         (message "%9s  %S"
+                         (message "%9s  %S%s"
                                   (ert-string-for-test-result result nil)
-                                  (ert-test-name test))))
+                                  (ert-test-name test)
+                                  (ert-reason-for-test-result result))))
               (message "%s" "")))))
        (test-started
         )
--
2.30.2

bug#47071: 28.0.50; Show ERT failure reason in batch mode

[Lars Ingebrigtsen]

Julian Scheid <jscheid@protonmail.com> writes:

> The data is only displayed in interactive mode as a side-effect
> however, not in batch mode.  I think it's useful to see the failure or
> skip reason in batch mode as well, which is why I would like to
> suggest the following change.

Thanks, but I'm not sure this level of detail is helpful in batch mode.

For instance, in python-tests, you get the wall of text included below,
which I think would be excessive.

Does anybody else have an opinion here?

14 skipped results:
  SKIPPED  python-shell-get-process-1  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-internal-get-or-create-process-1  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-make-comint-1  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-make-comint-2  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-make-comint-3  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-make-comint-4  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-detect-1  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-detect-2  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-detect-3  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-detect-4  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-detect-5  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-detect-6  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-shell-prompt-set-calculated-regexps-6  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)
  SKIPPED  python-tests--bug31398  ((skip-unless (executable-find python-tests-shell-interpreter)) :form (executable-find "python") :value nil)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47071: 28.0.50; Show ERT failure reason in batch mode

[Philipp Stephani]

Am Mo., 17. Mai 2021 um 17:36 Uhr schrieb Lars Ingebrigtsen <larsi@gnus.org>:
>
> Julian Scheid <jscheid@protonmail.com> writes:
>
> > The data is only displayed in interactive mode as a side-effect
> > however, not in batch mode.  I think it's useful to see the failure or
> > skip reason in batch mode as well, which is why I would like to
> > suggest the following change.
>
> Thanks, but I'm not sure this level of detail is helpful in batch mode.
>
> For instance, in python-tests, you get the wall of text included below,
> which I think would be excessive.
>
> Does anybody else have an opinion here?

On CI systems, you only have the logs written in batch mode. The more
complete they are, the better, since you can always filter out
information you don't need later, but not add missing information
after the fact. So printing lots of information is fine IMO.
A middle ground would be to support some kind of 'verbosity'
command-line option so that the environment could decide how much to
print.

bug#47071: 28.0.50; Show ERT failure reason in batch mode

[Lars Ingebrigtsen]

Philipp Stephani <p.stephani2@gmail.com> writes:

> On CI systems, you only have the logs written in batch mode. The more
> complete they are, the better, since you can always filter out
> information you don't need later, but not add missing information
> after the fact. So printing lots of information is fine IMO.

Yeah, that's true -- on CI systems the added information would be useful
indeed. 

> A middle ground would be to support some kind of 'verbosity'
> command-line option so that the environment could decide how much to
> print.

Yup.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47071: 28.0.50; Show ERT failure reason in batch mode

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> On CI systems, you only have the logs written in batch mode. The more
>> complete they are, the better, since you can always filter out
>> information you don't need later, but not add missing information
>> after the fact. So printing lots of information is fine IMO.
>
> Yeah, that's true -- on CI systems the added information would be useful
> indeed. 
>
>> A middle ground would be to support some kind of 'verbosity'
>> command-line option so that the environment could decide how much to
>> print.
>
> Yup.

I went with an environment variable to control this -- that seemed the
most flexible way here.  Feel free to tweak or change the name of the
variable.

Julian -- I pushed your patch to Emacs 28 with those small changes.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no