[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Kenichi Handa
Subject: Re: [PATCH] Re: running each test file independently in test/automated
Date: Mon, 16 Sep 2013 21:26:10 +0900

In article <address@hidden>, Barry OReilly <address@hidden> 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

> 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

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

        @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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]