* [PATCH] make test: Make failure results more verbose @ 2022-01-02 13:12 Ihor Radchenko 2022-01-03 5:35 ` Max Nikulin 0 siblings, 1 reply; 11+ messages in thread From: Ihor Radchenko @ 2022-01-02 13:12 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 158 bytes --] Hi, In newer Emacs, ERT is capable of providing more info about FAILED tests. Maybe we can enable this option by default in the Org test suite? Best, Ihor [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-make-test-Make-failure-results-more-verbose.patch --] [-- Type: text/x-diff, Size: 1607 bytes --] From 5e78a588a73871e54177080b3a6c9667f97500be Mon Sep 17 00:00:00 2001 Message-Id: <5e78a588a73871e54177080b3a6c9667f97500be.1641129033.git.yantar92@gmail.com> From: Ihor Radchenko <yantar92@gmail.com> Date: Sun, 2 Jan 2022 21:08:11 +0800 Subject: [PATCH] make test: Make failure results more verbose * mk/default.mk: New option BTEST_ERT_VERBOSE controlling verbosity of ERT results. The new default is verbose. * mk/targets.mk (check test test-dirty): Obey BTEST_ERT_VERBOSE Starting from https://git.savannah.gnu.org/cgit/emacs.git/commit/etc/NEWS?id=8be9d4a1568c34aed753b085d5d33daef5dfa797 ERT can output more verbose failure results. More verbose sounds like a better default. --- mk/default.mk | 2 ++ mk/targets.mk | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/mk/default.mk b/mk/default.mk index c8a15bdd2..c49950c04 100644 --- a/mk/default.mk +++ b/mk/default.mk @@ -32,6 +32,8 @@ TMPDIR ?= /tmp testdir = $(TMPDIR)/tmp-orgtest # Configuration for testing +# setup ERT vebosity +BTEST_ERT_VERBOSE = yes # add options before standard load-path BTEST_PRE = # add options after standard load path diff --git a/mk/targets.mk b/mk/targets.mk index 6de77b1e6..937bb82bc 100644 --- a/mk/targets.mk +++ b/mk/targets.mk @@ -103,7 +103,11 @@ vanilla: check test:: compile check test test-dirty:: -$(MKDIR) $(testdir) +ifeq ($(BTEST_ERT_VERBOSE),yes) + TMPDIR=$(testdir) EMACS_TEST_VERBOSE=yes $(BTEST) +else TMPDIR=$(testdir) $(BTEST) +endif ifeq ($(TEST_NO_AUTOCLEAN),) # define this variable to leave $(testdir) around for inspection $(MAKE) cleantest endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-02 13:12 [PATCH] make test: Make failure results more verbose Ihor Radchenko @ 2022-01-03 5:35 ` Max Nikulin 2022-01-07 15:04 ` Ihor Radchenko 0 siblings, 1 reply; 11+ messages in thread From: Max Nikulin @ 2022-01-03 5:35 UTC (permalink / raw) To: emacs-orgmode On 02/01/2022 20:12, Ihor Radchenko wrote: > > In newer Emacs, ERT is capable of providing more info about FAILED > tests. Maybe we can enable this option by default in the Org test suite? I like you attempts to make tests better. 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. In my opinion, code of test should be written having clear error reports in mind. > +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 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. > + 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. Shouldn't it be mentioned in testing/README? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-03 5:35 ` Max Nikulin @ 2022-01-07 15:04 ` Ihor Radchenko 2022-01-11 16:46 ` Max Nikulin 0 siblings, 1 reply; 11+ messages in thread From: Ihor Radchenko @ 2022-01-07 15:04 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-07 15:04 ` Ihor Radchenko @ 2022-01-11 16:46 ` Max Nikulin 2022-01-15 12:52 ` Max Nikulin 0 siblings, 1 reply; 11+ messages in thread From: Max Nikulin @ 2022-01-11 16:46 UTC (permalink / raw) To: emacs-orgmode On 07/01/2022 22:04, Ihor Radchenko wrote: > Max Nikulin 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: Thank you. After sending that message I realized that I did not try to load just ert.el from git to emacs version that I have installed. >> 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. Your patch improves batch error reporting. Good error message should allow to figure out the origin of the problem without interactive debugging. > Do you have good ideas what could be changed? Side note. In f0c474e659b81da0d2ab75e7ec109355965f7a1c I have noticed "* H1\nP1\n<point*H2\n" that likely should be "* H1\nP1\n<point>* H2\n" I do not remember where I have seen tests with rather ugly failure reports. Example of what I consider as minor improvement (sorry for the wrapped text): (ert-deftest tst-inline-tags-orig () (should (equal '("foo" "baz") (progn (require 'org-inlinetask) (org-test-with-temp-text (concat (make-string org-inlinetask-min-level ?*) " Test :foo:bar:") (org-get-tags (org-element-at-point))))))) (defun org-test-no-properties (arg) (cond ((stringp arg) (org-no-properties arg)) ((listp arg) (mapcar #'org-test-no-properties arg)) (t arg))) (ert-deftest tst-inline-tags-new () (let ((inline-stars (make-string org-inlinetask-min-level ?*))) (require 'org-inlinetask) (should (equal '("foo" "baz") (org-test-with-temp-text (concat inline-stars " Test :foo:bar:") (org-test-no-properties (org-get-tags (org-element-at-point)))))))) F tst-inline-tags-new (ert-test-failed ((should (equal '("foo" "baz") (org-test-with-temp-text (concat inline-stars " Test :foo:bar:") (org-test-no-properties ...)))) :form (equal ("foo" "baz") ("foo" "bar")) :value nil :explanation (list-elt 1 (array-elt 2 (different-atoms (122 "#x7a" "?z") (114 "#x72" "?r")))))) F tst-inline-tags-orig (ert-test-failed ((should (equal '("foo" "baz") (progn (require ...) (org-test-with-temp-text ... ...)))) :form (equal ("foo" "baz") (#("foo" 0 3 (keymap ... mouse-face highlight font-lock-fontified t face ...)) #("bar" 0 3 (keymap ... mouse-face highlight font-lock-fontified t face ...)))) :value nil :explanation (list-elt 1 (array-elt 2 (different-atoms (122 "#x7a" "?z") (114 "#x72" "?r")))))) >>> +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 "=". OK, maybe I will try it later. I just do not remember in which order assignments are evaluated. Likely everything works as expected and features like delayed assignment (simple "=" vs. ":=" ) do not cause any problem. >> 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 BTEST_RE is useful when you know in advance which tests you are interested in. I had in mind continuous integration and notifications. Single full error (or several ones) included into a message is convenient and may provide enough information for quick fix. If there are 500 errors there is no point to send all of them as notification. Either test is aborted after a couple of dozens of failures or total number of failures and full text of several errors are enough to avoid excessively long notification. Anyway nobody is going to read all hundreds of failures. >>> + 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. My bad. I did not noticed that it is a part of script of a rule. I believed that the whole line is assignment to TMPDIR variable. I am unsure if the following approach to avoid excessive ifeq's without modification of the rule is better: ifeq ($(BTEST_ERT_VERBOSE),yes) EMACS_TEST_VERBOSE=yes export EMACS_TEST_VERBOSE endif >> 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? I agree that default verbose log is preferred in most cases. The only exception when it becomes rather useless and just consumes disk space is something basic is broken (see my comment concerning limit on failures above). Let's assume that you convinced me and only rare developers will customize the value. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-11 16:46 ` Max Nikulin @ 2022-01-15 12:52 ` Max Nikulin 2022-01-15 13:06 ` Max Nikulin 2022-01-21 13:31 ` Ihor Radchenko 0 siblings, 2 replies; 11+ messages in thread From: Max Nikulin @ 2022-01-15 12:52 UTC (permalink / raw) To: emacs-orgmode Ihor, I have tried your patch. My opinion is that you can go ahead and commit it as is. Reaction to my comments is optional. > Subject: [PATCH] make test: Make failure results more verbose At first it was not clear to me that only *summary* of test results is affected. > +ifeq ($(BTEST_ERT_VERBOSE),yes) I would consider (it should be a single line) ifneq (,$(filter-out 0 n N no No NO f F false False FALSE,$(BTEST_ERT_VERBOSE))) to allow more variants to enable verbose logging, e.g. "Y", "true" or "1". The idea is that only empty string "", 0, "n", "no", "f", "false" values should disable verbose summary. On 11/01/2022 23:46, Max Nikulin wrote: > On 07/01/2022 22:04, Ihor Radchenko wrote: >> Max Nikulin writes: >> >> 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 COLUMNS environment variable may be a sensible default when set. It seems, some of variables are quite recent addition to Emacs, so some care is required. Maybe I am just not get used to such form of output, maybe I expect pretty print output instead of single form per line, maybe enough of call stack entries adds no value (e.g. pytest allows to suppress some frames from helper functions by defining special local variable and it cuts some outer frames by default). Actually some experience is required to quickly extract necessary information from backtrace. Even output of Python's pdb looks unfamiliar at first because gdb formats it in another way. > Thank you. After sending that message I realized that I did not try to > load just ert.el from git to emacs version that I have installed. It was more tricky than I expected. Latest ert is incompatible with Emacs-26. I am not motivated enough to build it from git, but compiling ert for 27 version generates no errors, just some warnings. >>> In my opinion, code of test should be written having clear error reports >>> in mind. > >> Do you have good ideas what could be changed? Consider `test-org-element/bold-parser' with a typo. Current summary: FAILED test-org-element/bold-parser ((should (equal (org-element-contents (org-test-with-temp-text "*first line\nsecond line*" (org-element-map ... ... ... nil t))) '("first line\nSecond line"))) :form (equal (#("first line\nsecond line" 0 22 (:parent (bold ... #3)))) ("first line\nSecond line")) :value nil :explanation (list-elt 0 (array-elt 11 (different-atoms (115 "#x73" "?s") (83 "#x53" "?S"))))) After some reorder (should (equal '("Line 1\nline 2") (org-test-with-temp-text "*Line 1\nline 2*" (org-element-contents (org-element-map (org-element-parse-buffer) 'bold #'identity nil t))))) it becomes a bit easy to read since expected value and input are closer to beginning: FAILED test-org-element/bold-parser ((should (equal '("Line 1\nline 2") (org-test-with-temp-text "*Line 1\nLine 2*" (org-element-contents (org-element-map ... ... ... nil t))))) :form (equal ("Line 1\nline 2") (#("Line 1\nLine 2" 0 13 (:parent (bold ... #3))))) :value nil :explanation (list-elt 0 (array-elt 7 (different-atoms (108 "#x6c" "?l") (76 "#x4c" "?L"))))) Actually I am a bit surprised that `different-atoms' are shortened to "..." in long failure report but readable in summary. > Side note. In f0c474e659b81da0d2ab75e7ec109355965f7a1c I have noticed > "* H1\nP1\n<point*H2\n" > that likely should be > "* H1\nP1\n<point>* H2\n" Unrelated to this patch, but still should be fixed. >>> 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 "=". It was a false alarm, sorry for it. local.mk included after default.mk, so there is no problem to override value of BTEST_ERT_VERBOSE. >>>> + 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. > > My bad. I did not noticed that it is a part of script of a rule. I > believed that the whole line is assignment to TMPDIR variable. I am > unsure if the following approach to avoid excessive ifeq's without > modification of the rule is better: > > ifeq ($(BTEST_ERT_VERBOSE),yes) > EMACS_TEST_VERBOSE=yes > export EMACS_TEST_VERBOSE > endif With such approach the variable is exported for all targets, not only for testing ones, however it will not harm. The advantage of your variant is that value of the variable value may be found in logs, disadvantage is that makefile is a bit more obscure. >>> 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? > > I agree that default verbose log is preferred in most cases. The only > exception when it becomes rather useless and just consumes disk space is > something basic is broken (see my comment concerning limit on failures > above). Let's assume that you convinced me and only rare developers will > customize the value. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-15 12:52 ` Max Nikulin @ 2022-01-15 13:06 ` Max Nikulin 2022-01-15 15:58 ` Max Nikulin 2022-01-21 13:31 ` Ihor Radchenko 1 sibling, 1 reply; 11+ messages in thread From: Max Nikulin @ 2022-01-15 13:06 UTC (permalink / raw) To: emacs-orgmode A couple of additional remarks. On 15/01/2022 19:52, Max Nikulin wrote: > On 02/01/2022 20:12, Ihor Radchenko wrote: > >> Subject: [PATCH] make test: Make failure results more verbose > > At first it was not clear to me that only *summary* of test results is > affected. Should not the variable be named BTEST_ERT_VERBOSE_SUMMARY instead of just BTEST_ERT_VERBOSE due to this reason? >>>> 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 "=". I have found an example when "?=" is better. It is possible to export BTEST_ERT_VERBOSE from shell (or set in in the environment of continuous integration runner) and the value will not be overridden. Quick tests shell equivalent: BTEST_ERT_VERBOSE= make test currently it have to be written only as make test-dirty BTEST_ERT_VERBOSE= ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-15 13:06 ` Max Nikulin @ 2022-01-15 15:58 ` Max Nikulin 2022-01-21 13:33 ` Ihor Radchenko 0 siblings, 1 reply; 11+ messages in thread From: Max Nikulin @ 2022-01-15 15:58 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1709 bytes --] On 02/01/2022 20:12, Ihor Radchenko wrote: > > In newer Emacs, ERT is capable of providing more info about FAILED > tests. Maybe we can enable this option by default in the Org test suite? Thinking more, I have realized that something is wrong. Behavior of tests in Org should be controlled by EMACS_TEST_VERBOSE *environment* variable. Org makefiles may have it on by default, but it should not override explicitly chosen value. Fortunately Emacs repository contains no makefiles from Org, so it will not cause conflict with Emacs builds. The problem is that EMACS_TEST_VERBOSE interface is broken. In the following case I expect that test summary should not be verbose: ert-sample.el ---- >8 ---- (require 'ert) (ert-deftest ert-sample () (should (equal 5 (* 2 2)))) (ert-run-tests-batch-and-exit) ---- 8< ---- (I am tried it with Emacs-27, so I put a copy of ert.el from git HEAD to ~/ert) EMACS_TEST_VERBOSE= emacs --batch -Q -L ~/ert -l ert-sample.el 1 unexpected results: FAILED ert-sample ((should (equal 5 (* 2 2))) :form (equal 5 4) :value nil :explanation (different-atoms (5 "#x5" "?") (4 "#x4" "?"))) That is why I am going to file a bug against ert. In a minimal variant empty string (`getenv' return value) should not be considered as t. For better user experience, I expect that the following case-insensitive strings should be considered as false: "0", "false", "no", "n", "off", "f" (borrowed from .ini), "none", "nil". Other values should be considered as true or anything besides "1", "true", "yes", "y", "on", "t" should be considered as invalid value. I am attaching a tentative patch for Org that should make EMACS_TEST_VERBOSE setting more transparent. [-- Attachment #2: 0001-make-test-Make-failure-summary-more-verbose.patch --] [-- Type: text/x-patch, Size: 1654 bytes --] From 5f8d93a9895b67ce89bac3b2d95ca723fe754ab4 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Sat, 15 Jan 2022 22:54:30 +0700 Subject: [PATCH] make test: Make failure summary more verbose * mk/default.mk: By default enable verbose failure summary for Emacs-28 or newer. Set or unset EMACS_TEST_VERBOSE environment to control reporting of failure reasons in summary since ERT switches to verbose mode even by an empty string. As an extension, allow values as "0", "off", "no" to disable verbosity. --- mk/default.mk | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/mk/default.mk b/mk/default.mk index c8a15bdd2..11ddcc2e1 100644 --- a/mk/default.mk +++ b/mk/default.mk @@ -32,6 +32,24 @@ TMPDIR ?= /tmp testdir = $(TMPDIR)/tmp-orgtest # Configuration for testing +# Verbose ERT summary by default for Emacs-28 and above. +# To override: +# - Add to local.mk +# EMACS_TEST_VERBOSE = +# - Export EMACS_TEST_VERBOSE environment variable with empty value +# - Run tests as +# EMACS_TEST_VERBOSE= make test [OTHER_ARGUMENTS...] +# or as +# make test EMACS_TEST_VERBOSE= [OTHER_ARGUMENTS...] +# For convenience some other values are recognized as false by Org makefiles +# ("0", "no", "off", "false") despite any set value (including empty one) +# is considered as true by ERT. +EMACS_TEST_VERBOSE ?= yes +ifneq (,$(filter-out 0 n N no No NO f F false False FALSE off Off OFF none None NONE nil NIL,$(EMACS_TEST_VERBOSE))) +export EMACS_TEST_VERBOSE +else +unexport EMACS_TEST_VERBOSE +endif # add options before standard load-path BTEST_PRE = # add options after standard load path -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-15 15:58 ` Max Nikulin @ 2022-01-21 13:33 ` Ihor Radchenko 2022-01-21 15:01 ` Max Nikulin 0 siblings, 1 reply; 11+ messages in thread From: Ihor Radchenko @ 2022-01-21 13:33 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 368 bytes --] Max Nikulin <manikulin@gmail.com> writes: > I am attaching a tentative patch for Org that should make > EMACS_TEST_VERBOSE setting more transparent. Thanks! Your patch looks cleaner. In addition, I am attaching a tentative hack to make ERT pretty-print the failure reason as you wished. WDYT? Though it may also be something to consider in Emacs core. Best, Ihor [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-make-test-Pretty-print-failure-reason-in-test-summar.patch --] [-- Type: text/x-patch, Size: 1745 bytes --] From 1a8520b9cb2883672b1a6d2385d539a158658890 Mon Sep 17 00:00:00 2001 Message-Id: <1a8520b9cb2883672b1a6d2385d539a158658890.1642771846.git.yantar92@gmail.com> From: Ihor Radchenko <yantar92@gmail.com> Date: Fri, 21 Jan 2022 21:30:21 +0800 Subject: [PATCH] make test: Pretty print failure reason in test summary --- testing/org-test.el | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/testing/org-test.el b/testing/org-test.el index 0f1e254aa..a00007b6b 100644 --- a/testing/org-test.el +++ b/testing/org-test.el @@ -344,6 +344,13 @@ (defun org-test-strip-text-props (s) (let ((noprop (copy-sequence s))) (set-text-properties 0 (length noprop) nil noprop) noprop)) + +(require 'cl-macs) +(defun org-test-ert-reason-for-test-result:pretty-func (func result) + "Call `ert-reason-for-test-result' (as FUNC(RESULT)). +Pretty-print the failure reason." + (cl-letf (((symbol-function 'format) (lambda (_ reason) (concat "\n" (pp-to-string reason))))) + (funcall func result))) \f (defun org-test-string-exact-match (regex string &optional start) @@ -435,7 +442,11 @@ (defun org-test-run-batch-tests (&optional org-test-selector) (org-test-update-id-locations) (org-test-load) (message "selected tests: %s" org-test-selector) - (ert-run-tests-batch-and-exit org-test-selector))) + (advice-add 'ert-reason-for-test-result :around + #'org-test-ert-reason-for-test-result:pretty-func) + (ert-run-tests-batch-and-exit org-test-selector) + (advice-remove 'ert-reason-for-test-result + #'org-test-ert-reason-for-test-result:pretty-func))) (defun org-test-run-all-tests () "Run all defined tests matching \"\\(org\\|ob\\)\". -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-21 13:33 ` Ihor Radchenko @ 2022-01-21 15:01 ` Max Nikulin 2022-01-23 13:31 ` Ihor Radchenko 0 siblings, 1 reply; 11+ messages in thread From: Max Nikulin @ 2022-01-21 15:01 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 2078 bytes --] On 21/01/2022 20:33, Ihor Radchenko wrote: > Max Nikulin <manikulin@gmail.com> writes: > >> I am attaching a tentative patch for Org that should make >> EMACS_TEST_VERBOSE setting more transparent. > > Thanks! Your patch looks cleaner. In the meanwhile Lars fixed ERT in Emacs-29, so empty string is considered as false now, see https://debbugs.gnu.org/53313 Values like "no" should not be considered as false by Org makefiles, so I am attaching an updated version. > In addition, I am attaching a tentative hack to make ERT pretty-print > the failure reason as you wished. WDYT? Sorry, I was not clear enough. Summary should remain single-line. There are pretty-printed failure reason in the main part of the log. I was complained concerning the following brick > signal(ert-test-failed (((should (equal "aB " (org-test-with-parsed- > ert-fail(((should (equal "aB " (org-test-with-parsed-data "* Headlin > (if (unwind-protect (setq value-5061 (apply fn-5059 args-5060)) (set > (let (form-description-5063) (if (unwind-protect (setq value-5061 (a > (let ((value-5061 'ert-form-evaluation-aborted-5062)) (let (form-des > (let* ((fn-5059 #'equal) (args-5060 (condition-case err (let ((signa > (closure (t) nil (let* ((fn-5059 #'equal) (args-5060 (condition-case > ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test > ert-run-test(#s(ert-test :name test-org-export/get-previous-element > ert-run-or-rerun-test(#s(ert--stats :selector "test-org-export/get-p > ert-run-tests("test-org-export/get-previous-element" #f(compiled-fun > ert-run-tests-batch("test-org-export/get-previous-element") > ert-run-tests-batch-and-exit("test-org-export/get-previous-element") > (let ((org-id-track-globally t) (org-test-selector (if org-test-sele > org-test-run-batch-tests("test-org-export/get-previous-element") > eval((org-test-run-batch-tests org-test-select-re) t) > command-line-1(("-L" "/home/ubuntu/ert" "--eval" "(setq vc-handled-b > command-line() > normal-top-level() However it will be too long when pretty printed. [-- Attachment #2: 0001-make-test-Make-failure-summary-more-verbose.patch --] [-- Type: text/x-patch, Size: 1365 bytes --] From a53095fa1c6679376708dcc1a05a446c3967d914 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Sat, 15 Jan 2022 22:54:30 +0700 Subject: [PATCH] make test: Make failure summary more verbose * mk/default.mk: By default enable verbose failure summary for Emacs-28 or newer. Set or unset EMACS_TEST_VERBOSE environment to control reporting of failure reasons in summary since in Emacs-28 ERT switches to verbose mode even by an empty string (fixed in Emacs-29). --- mk/default.mk | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/mk/default.mk b/mk/default.mk index c8a15bdd2..804089280 100644 --- a/mk/default.mk +++ b/mk/default.mk @@ -32,6 +32,20 @@ TMPDIR ?= /tmp testdir = $(TMPDIR)/tmp-orgtest # Configuration for testing +# Verbose ERT summary by default for Emacs-28 and above. +# To override: +# - Add to local.mk +# EMACS_TEST_VERBOSE = +# - Export EMACS_TEST_VERBOSE environment variable with empty value +# - Run tests as +# EMACS_TEST_VERBOSE= make test [OTHER_ARGUMENTS...] +# or as +# make test EMACS_TEST_VERBOSE= [OTHER_ARGUMENTS...] +EMACS_TEST_VERBOSE ?= yes +ifeq (,$(EMACS_TEST_VERBOSE)) +# Emacs-28 considers empty value as true, fixed in Emacs-29 +unexport EMACS_TEST_VERBOSE +endif # add options before standard load-path BTEST_PRE = # add options after standard load path -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-21 15:01 ` Max Nikulin @ 2022-01-23 13:31 ` Ihor Radchenko 0 siblings, 0 replies; 11+ messages in thread From: Ihor Radchenko @ 2022-01-23 13:31 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >> Thanks! Your patch looks cleaner. > > In the meanwhile Lars fixed ERT in Emacs-29, so empty string is > considered as false now, see https://debbugs.gnu.org/53313 > Values like "no" should not be considered as false by Org makefiles, so > I am attaching an updated version. Looks good. I applied your patch on main as 2da104643. >> In addition, I am attaching a tentative hack to make ERT pretty-print >> the failure reason as you wished. WDYT? > > Sorry, I was not clear enough. Summary should remain single-line. There > are pretty-printed failure reason in the main part of the log. I was > complained concerning the following brick Got it. Make sense. Best, Ihor ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] make test: Make failure results more verbose 2022-01-15 12:52 ` Max Nikulin 2022-01-15 13:06 ` Max Nikulin @ 2022-01-21 13:31 ` Ihor Radchenko 1 sibling, 0 replies; 11+ messages in thread From: Ihor Radchenko @ 2022-01-21 13:31 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >> Side note. In f0c474e659b81da0d2ab75e7ec109355965f7a1c I have noticed >> "* H1\nP1\n<point*H2\n" >> that likely should be >> "* H1\nP1\n<point>* H2\n" > > Unrelated to this patch, but still should be fixed. Thanks! Fixed. Best, Ihor ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-01-23 13:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-02 13:12 [PATCH] make test: Make failure results more verbose Ihor Radchenko 2022-01-03 5:35 ` Max Nikulin 2022-01-07 15:04 ` Ihor Radchenko 2022-01-11 16:46 ` Max Nikulin 2022-01-15 12:52 ` Max Nikulin 2022-01-15 13:06 ` Max Nikulin 2022-01-15 15:58 ` Max Nikulin 2022-01-21 13:33 ` Ihor Radchenko 2022-01-21 15:01 ` Max Nikulin 2022-01-23 13:31 ` Ihor Radchenko 2022-01-21 13:31 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).