Re: [PATCH] make test: Make failure results more verbose

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> Ihor, are there examples of new error reports in mail lists, blogs, etc? 
> I am not motivated enough to try development version of emacs, but my 
> impression that current error log looks like rectangles of garbage.

Yeah, I should have attached examples to the original message.
Also, we can adjust the ERT output using
ert-batch-backtrace-right-margin, ert-batch-print-level,
ert-batch-print-level, and ert-batch-backtrace-line-length

Here are the examples:

1 unexpected results:
   FAILED  test-org-element/center-block-parser  "This error is thrown"
vs
1 unexpected results:
   FAILED  test-org-element/center-block-parser

and

1 unexpected results:
   FAILED  test-org-element/bold-parser  ((should (org-test-with-temp-text 
"*bold *" (org-element-map (org-element-parse-buffer) 'bold #'identity nil t))) 
:form (let ((inside-text (if (stringp "*bold *") "*bold *" (eval "*bold *"))) 
(org-mode-hook nil)) (let ((temp-buffer (generate-new-buffer " *temp*" t))) 
(save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn ... ... 
... ...) (and ... ...))))) :value nil)
vs
1 unexpected results:
   FAILED  test-org-element/bold-parser

and

1 unexpected results:
   FAILED  test-org-element/bold-parser  ((should (equal (org-element-contents 
(org-test-with-temp-text "*first line\nsecond blah line*" (org-element-map ... 
... ... nil t))) '("first line\nsecond line"))) :form (equal (#("first 
line\nsecond blah line" 0 27 (:parent (bold ... #3)))) ("first line\nsecond 
line")) :value nil :explanation (list-elt 0 (arrays-of-different-length 27 22 
#("first line\nsecond blah line" 0 27 (:parent (bold ... #3))) "first 
line\nsecond line" first-mismatch-at 18)))

and

1 unexpected results:
   FAILED  test-org-element/citation-parser  ((should (equal '("a" "b" "c") 
(org-test-with-temp-text "[cite:@a;@bd;@c]" (org-element-map 
(org-element-parse-buffer) 'citation-reference (lambda ... ...))))) :form 
(equal ("a" "b" "c") ("a" "bd" "c")) :value nil :explanation (list-elt 1 
(arrays-of-different-length 1 2 "b" "bd" first-mismatch-at 1)))

> In my opinion, code of test should be written having clear error reports 
> in mind.

I guess. Though I feel that ERT is better when used interactively.
Do you have good ideas what could be changed?

>> +BTEST_ERT_VERBOSE = yes
>
> I am unsure if this line or local.mk has priority. I am unsure the the 
> following is better as well.
> BTEST_ERT_VERBOSE ?= yes

I am not very familiar with Makefile conventions. Just followed the
existing settings in the same file. All other BTEST_ERT_* settings just
use "=".

> Is there an easy way to limit number of failures before termination of 
> tests in the case of verbose reporting? It should prevent test log from 
> blowing too much. Usually there is no point in all details if all or 
> even 1/4 of tests fails.

The best approach I know is using BTEST_RE

>> +    TMPDIR=$(testdir) EMACS_TEST_VERBOSE=yes $(BTEST)
>
> A purist would say that it is not a directory, it is something like 
> ...FLAGS or ...ARGS. I know, it was abused before your patch.

I do not follow you here.
VARIABLE1=1 VARIABLE2=2 /path/to/script
is the usual way to set variables in script environment in bash.

> Shouldn't it be mentioned in testing/README?

Only BTEST_RE is documented there. BTEST_PRE, BTEST_POST,
BTEST_OB_LANGUAGES, and BTEST_EXTRA are not documented.
I believe that the odds for the user to change BTEST_ERT_VERBOSE are
rather low and it is not worth mentioning it explicitly in README.

Or, alternatively, we can document all the settings in README.
WDYT?

Best,
Ihor