[PATCH] Re: running each test file independently in test/automated

[PATCH] Re: running each test file independently in test/automated

[Barry OReilly]

[-- Attachment #1: Type: text/plain, Size: 3993 bytes --]

>>> Hi, I've found I want to run only one or a few of the tests at a
>>> time rather than the whole suite.

>> http://lists.gnu.org/archive/html/emacs-devel/2013-08/msg00339.html
>>
>> is supposed to allow this. I don't know if it is waiting on
>> something.

> Yes indeed, it provides a way to run individual tests:
>
>   $ rm cl-lib.log ; make cl-lib.log
>   Running tests in cl-lib.el... passed all 8 tests
>
> Some comments on the patch follow.

I haven't heard from Kenichi, so I prepared a patch to implement the
subset of what he submitted pertaining to the .log make targets.

I also factored out some duplication in the compile-main,
compile-clean, and check targets.

I'll plan to rename the Makefile to GNUmakefile as Stefan requested in
the same commit, once the patch below is approved for install.

diff --git a/.bzrignore b/.bzrignore
index cd64f3f..be86add 100644
--- a/.bzrignore
+++ b/.bzrignore
@@ -174,6 +174,7 @@ src/stamp-oldxmenu
 src/stamp-h.in
 src/temacs
 test/indent/*.new
+test/automated/**/*.log
 +*
 src/globals.h
 src/gl-stamp
diff --git a/test/automated/Makefile.in b/test/automated/Makefile.in
index bf8e62f..3005d5e 100644
--- a/test/automated/Makefile.in
+++ b/test/automated/Makefile.in
@@ -49,12 +49,16 @@ BYTE_COMPILE_EXTRA_FLAGS =
 emacs = EMACSLOADPATH=$(lispsrc):$(test) LC_ALL=C $(EMACS) $(EMACSOPT)

 # Common command to find subdirectories
-setwins=subdirs=`find . -type d -print`; \
+getwins=subdirs=`find . -type d -print`; \
     for file in $$subdirs; do \
        case $$file in */.* | */.*/* | */=* | ./data* ) ;; \
         *) wins="$$wins$${wins:+ }$$file" ;; \
        esac; \
-        done
+        done; \
+    echo "$$wins" | sed -e 's|/\./|/|g' -e 's|/\. | |g'
+
+TEST_EL_FILES=$(wildcard $(patsubst %,%/*.el,$(shell $(getwins))))
+TEST_ELC_FILES=$(wildcard $(patsubst %,%/*.elc,$(shell $(getwins))))

 all: check

@@ -68,7 +72,7 @@ doit:
 # subdirectories, to make sure require's and load's in the files being
 # compiled find the right files.

-.SUFFIXES: .elc .el
+.SUFFIXES: .elc .el .log

 # An old-fashioned suffix rule, which, according to the GNU Make manual,
 # cannot have prerequisites.
@@ -98,9 +102,8 @@ compile-targets: $(TARGETS)
 # Compile all the Elisp files that need it.  Beware: it approximates
 # `no-byte-compile', so watch out for false-positives!
 compile-main: compile-clean lisp-compile
-    @(cd $(test); $(setwins); \
-    els=`echo "$$wins " | sed -e 's|/\./|/|g' -e 's|/\. | |g' -e 's|
|/*.el |g'`; \
-    for el in $$els; do \
+    @(cd $(test); \
+    for el in $(TEST_EL_FILES); do \
       test -f $$el || continue; \
       test ! -f $${el}c && GREP_OPTIONS= grep '^;.*no-byte-compile: t'
$$el > /dev/null && continue; \
       echo "$${el}c"; \
@@ -112,9 +115,8 @@ compile-main: compile-clean lisp-compile
 .PHONY: compile-clean
 # Erase left-over .elc files that do not have a corresponding .el file.
 compile-clean:
-    @cd $(test); $(setwins); \
-    elcs=`echo "$$wins " | sed -e 's|/\./|/|g' -e 's|/\. | |g' -e 's|
|/*.elc |g'`; \
-    for el in $$(echo $$elcs | sed -e 's/\.elc/\.el/g'); do \
+    @cd $(test); \
+    for el in $(patsubst %.elc,%.el,$(TEST_ELC_FILES)); do \
       if test -f "$$el" -o \! -f "$${el}c"; then :; else \
         echo rm "$${el}c"; \
         rm "$${el}c"; \
@@ -145,10 +147,18 @@ distclean:

 maintainer-clean: distclean bootstrap-clean

+TEST_LOGS=$(patsubst %.el,%.log,$(TEST_EL_FILES))
+$(TEST_LOGS): lisp-compile
+$(TEST_LOGS): %.log : %.elc
+    @(cd $(test); \
+    el=$(patsubst %.log,%.el,$@) ;\
+    test -f $$el || continue; \
+    echo Testing $$el; \
+    $(emacs) -l $$el -f ert-run-tests-batch-and-exit 2>&1 | tee $@)
+
 check: compile-main
-    @(cd $(test); $(setwins); \
-    pattern=`echo "$$wins " | sed -e 's|/\./|/|g' -e 's|/\. | |g' -e 's|
|/*.el |g'`; \
-    for el in $$pattern; do \
+    @(cd $(test); \
+    for el in $(TEST_EL_FILES); do \
       test -f $$el || continue; \
       args="$$args -l $$el"; \
       els="$$els $$el"; \

[-- Attachment #2: Type: text/html, Size: 4859 bytes --]

Re: [PATCH] Re: running each test file independently in test/automated

[Kenichi Handa]

In article <CAFM41H0xme+mHk53ROj9VEmtSQs1aasf2A8TGdX=3rnkwos1Kw@mail.gmail.com>, Barry OReilly <gundaetiapo@gmail.com> writes:

> I haven't heard from Kenichi, so I prepared a patch to implement the
> subset of what he submitted pertaining to the .log make targets.

I'm very sorry for no response.  I didn't receive your mail
(and the previous Stefan's mail for allowing to commit my
change).  I might have fetched mails on temporal virtual
machine by mistake.  :-(

> > Some comments on the patch follow.

Thank you for them.  Here are replies.

> > TEST_LOGS = $(patsubst %.el, %.log, $(wildcard $(test)/*.el))

> Other recipes in the same Makefile determine the set of .el files a
> different way: they include .el files in subdirectories except data/.
> There aren't actually such .el files, but the moment someone adds one
> the make code is inconsistent.

You are right.  I was just lazy.

> >     @test -d `dirname "$@"` || mkdir `dirname "$@"`

> Why not: mkdir -p `dirname "$@"`

I was not sure how how portable it is.

> > parallel: $(TEST_LOGS)
> >     @cd $(test); $(emacs) -f ert-summary-report $(TEST_LOGS)

> Instead of creating the new "parallel" target, could we just have the
> "check" target run the tests individually?

> One argument against might be that a -j1 build would be longer. Here
> are some benchmarks (2 CPU cores).

Yes.  If one want to run all tests again on a single core
CPU machine, "make check" is faster than "make -j clean
parallel"

> If however we keep the parallel target, it should be renamed. It seems
> off to name a target "parallel" just because it is parallelizable. If
> the user doesn't pass -j then the target name is technically
> incorrect. "summary" would be a good name given what it does.

I agree that "parallel" was not a good name.  I'm ok with
"summary".

> > (defun ert-run-tests-batch-and-exit-single ()
> > [...]
> >          ;; Load a byte-compiled one or TEST-FILE itself.
> >          (if (file-newer-than-file-p compiled test-file)
> >              (progn
> >                (setq base (file-name-nondirectory compiled))
> >                (load-file compiled))
> >            (let ((buf (find-file-noselect test-file)))
> >              (if (with-current-buffer buf
> >                    (and (boundp 'no-byte-compile) no-byte-compile))
> >                  (with-current-buffer buf
> >                    (eval-buffer))
> >                (if (byte-compile-file test-file t)
> >                    (setq base (file-name-nondirectory compiled))
> >                  (princ (format "%s failed to compile the file\n" prefix))
> >                  (message "##REPORT:(compile-error \"%s\")##" base)
> >                  (kill-emacs 0))))

> Why shouldn't Make have compiled the test-file? Perhaps the log files
> should depend on the .elc files instead of the .el files.

We have to catch an error of byte-compilation, but there
already exists this target and rule, and I'd like not to
change the original behavior.

.el.elc:
	@echo Compiling $<
	@$(emacs) $(BYTE_COMPILE_EXTRA_FLAGS) -f batch-byte-compile $<

> > (defun ert-run-tests-batch-and-exit-single ()
> > [...]
> >                  (message "##REPORT:(compile-error \"%s\")##" base)
> > [...]
> >            (message "##REPORT:(done %d %d)##" total expected)
> > [...]
> >       (message "##REPORT:(load-error \"%s\")##" base)

> It seems the only reason to have ert-run-tests-batch-and-exit-single
> is to insert these "##REPORT" tokens. But why can't ert-summary-report
> parse:

>   '^Ran \([0-9]*\) tests, \([0-9]*\) results as expected'

> to get the same information? Then could you remove the
> ert-run-tests-batch-and-exit-single function and invoke the existing
> ert-run-tests-batch-and-exit?

As I wrote about, I'd like to byte-compile test files in
this function, and if we make such a function, I thought
it's easier to generate all kinds of ##REPORT in it.

> > (defun ert-summary-report ()
> > [...]
> >     (when errors
> >       (message "\n  Following test files have problems:")

> When I ran the parallel target, I didn't get this message at all, even
> though I have some test failures. eg from my file-notify-tests.log:

>   1 unexpected results:
>      FAILED  file-notify-test00-availability

It means there is no error (byte-compiling, loading, running) in
test files themselves.

---
Kenichi Handa
handa@gnu.org

Re: [PATCH] Re: running each test file independently in test/automated

[Barry OReilly]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

> We have to catch an error of byte-compilation,

To what end? The individual can see build errors and I presume Hydra
reports build failures originating in the Makefile.

> but there already exists this target and rule, and I'd like not to
> change the original behavior.

Doing in ert.el what the Makefile is responsible for seems off to me.
If test code build errors are to go in the summary, why shouldn't
build errors of the code under test? If incorporating build failures
into a summary report is good enough to do, why isn't it good enough
for 'make check'?

[-- Attachment #2: Type: text/html, Size: 665 bytes --]

Re: [PATCH] Re: running each test file independently in test/automated

[Kenichi Handa]

In article <CAFM41H3JOzvyOUf_eRYj+MmD3X4f6n8-qohGpzyZRwm-FacHgw@mail.gmail.com>, Barry OReilly <gundaetiapo@gmail.com> writes:

> > We have to catch an error of byte-compilation,

> To what end? The individual can see build errors and I presume Hydra
> reports build failures originating in the Makefile.

Ah, "have to" was too strong, but it is more convenient that
the whole testing doesn't stop by an error of a single test
file.

> > but there already exists this target and rule, and I'd like not to
> > change the original behavior.

> Doing in ert.el what the Makefile is responsible for seems off to me.

I agree with that.  If we don't have to keep the original
behavior of "make check", let's modify the rule of
".el.elc:".

> If test code build errors are to go in the summary, why shouldn't
> build errors of the code under test? If incorporating build failures
> into a summary report is good enough to do, why isn't it good enough
> for 'make check'?

Sorry but I don't understand what you want to say.

---
Kenichi Handa
handa@gnu.org

Re: [PATCH] Re: running each test file independently in test/automated

[Barry OReilly]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

> it is more convenient that the whole testing doesn't stop by an
> error of a single test file.

> If we don't have to keep the original behavior of "make check",
> let's modify the rule of ".el.elc:".

So the test code byte compilation rule would not stop Make, but would
be captured in the .log file and reported at the end summary report?
That is fine to me.

We should have ert-summary-report kill-emacs with positive error code
if there were such errors. That way the overall 'make check' will
still fail if there were build errors.

[-- Attachment #2: Type: text/html, Size: 669 bytes --]

Re: [PATCH] Re: running each test file independently in test/automated

[Kenichi Handa]

In article <CAFM41H1YM2p6-LLf993aSJQTkaQcm2mUdRMgf8VCZA6BTcew9w@mail.gmail.com>, Barry OReilly <gundaetiapo@gmail.com> writes:

> So the test code byte compilation rule would not stop Make, but would
> be captured in the .log file and reported at the end summary report?

Yes.  Making each test independent is the most important
feature of my patch.  A bug in a test code doesnt't stop
Make.  Even if emacs dumps core, Make doesn't stop.

> We should have ert-summary-report kill-emacs with positive error code
> if there were such errors. That way the overall 'make check' will
> still fail if there were build errors.

Ah, yes, you are right.

---
Kenichi Handa
handa@gnu.org

Re: [PATCH] Re: running each test file independently in test/automated

[Glenn Morris]

Kenichi Handa wrote:

>> >     @test -d `dirname "$@"` || mkdir `dirname "$@"`
>
>> Why not: mkdir -p `dirname "$@"`
>
> I was not sure how how portable it is.

Configure sets @MKDIR_P@ to an appropriate value.