Re: patch to add AT_CHECK_EUNIT autotest macro

[Romain Lenglet]

Hi Ralf,

Thanks for your comments.
I attached an updated patch which addresses your comments, except the 
conditional definition of variables in atconfig.

On Saturday 01 August 2009 14:26:11 Ralf Wildenhues wrote:
> the idea seems ok to me.  Please test distcheck before committing.

I checked it passes.

> AT_CHECK has measures to complain if it is not between AT_SETUP and
> AT_CLEANUP.  Can you please check that AT_CHECK_EUNIT complains, too
> (it should, because it uses AT_CHECK)?  Thanks.

AT_CHECK_EUNIT is defined using _AT_DEFINE_SETUP, like AT_CHECK.
I checked that it fails if not between AT_SETUP and AT_CLEANUP.

> Would users want to write tests that are expected to fail/exit with a
> nonzero status (from one of the involved tools)?

EUnit is a library, which doesn't exit the process in any case, and unit tests 
executed by EUnit are not expected to exit the process either. For EUnit, 
either a test passes or it fails. There is no other information returned 
(except for information output on stdout). So AT_CHECK_EUNIT generates and 
execute a "wrapper" Erlang module that calls the EUnit library with the given 
test spec, and returns an appropriate exit status depending on the result of 
the test:
- 77 if EUnit is not found;
- 1 if the test fails;
- 0 if the test passes.
There's no need and no way to distinguish other cases.

> > +If the testsuite is run in verbose mode, with option @option{--verbose},
> > +EUnit is also run in verbose mode to output more details about
> > +individual unittests.
>
> s/unittests/unit tests/  ?   (more instances below)

OK, done.

> > --- a/lib/autoconf/autotest.m4
> > +++ b/lib/autoconf/autotest.m4
> > @@ -84,6 +84,14 @@ at_top_builddir=\$at_top_build_prefix
> >  AUTOTEST_PATH='m4_default([$2], [$1])'
> >
> >  SHELL=\${CONFIG_SHELL-'$SHELL'}
> > +
> > +# Required to run EUnit unittests.
> > +ERL='$ERL'
> > +ERLC='$ERLC'
> > +ERLCFLAGS='$ERLCFLAGS'
> >  ATEOF
> > -])
> > +],
> > +[ERL="$ERL"
> > +ERLC="$ERLC"
> > +ERLCFLAGS="$ERLCFLAGS"])
>
> Can we make this code addition conditional to test suites in which
> AT_CHECK_EUNIT is used?  (Diversions ought to help here.)
> We don't want unnecessary code expansions.

I don't see how to do that easily, since AT_CONFIG_TESTDIR doesn't parse the 
test suite at all. How can I detect, in configure, the use of a macro in a 
testsuite? We would have to do something like AT_INIT_AUTOMAKE, i.e. have the 
testsuite generation also produce aclocal macros like Automake?

It should be better to make the definition of those variables conditional to 
the use of AC_ERLANG_PATH_ERL and AC_ERLANG_PATH_ERLC in configure.ac, as it 
should be the normal way of defining those variables?

> > +_AT_DEFINE_SETUP([AT_CHECK_EUNIT],
> > +[AT_CHECK([test -f "$ERL" && test -f "$ERLC" || exit 77])
>
> AT_SKIP_IF has lower overhead.

OK, replaced with:
AT_SKIP_IF([test ! -f "$ERL" || test ! -f "$ERLC"])

> > +AT_CHECK(["$ERLC" $ERLCFLAGS -b beam $1.erl])
>
> Does that never produce any output?  Wow.

If compilation is fine, no output is produced. But it outputs something in 
case of warnings or errors, which in this case indicates that the test-spec 
has an invalid syntax, which should fail the test. I think that this is a 
legitimate use of AT_CHECK?

> > +AT_CHECK([/bin/sh ./compile])
>
> $CONFIG_SHELL, please.

OK, done.

Thanks,
--
Romain Lenglet

0001-Add-AT_CHECK_EUNIT-macro-to-run-Erlang-EUnit-unittes.patch
Description: Text Data