Re: patch to add autotests for Erlang macros

[Romain Lenglet]

On Tuesday 21 July 2009 20:23:59 Eric Blake wrote:
> According to Romain Lenglet on 7/20/2009 10:33 PM:
> > Hi,
> >
> > The attached patch adds tests for the macros in erlang.m4.
>
> A big thanks for this - it is long overdue.  I don't have an erlang
> interpreter handy on my primary development machine; do you have a URL to
> a tarball so I can try building one and to this a bit more thoroughly?

There are packages for most distributions.
There are source tarballs there:
http://www.erlang.org/download.html
Any version downloadable from there is OK. All are stable.

> > I haven't been able to use AT_CHECK_MACRO for testing most of the
> > macros, because most macros fail in AT_CHECK_ENV, for reasons I don't
> > yet understand. It seems that every time a test that runs Erlang code is
> > peformed, AT_CHECK_ENV fails, so it may be a bug in the Erlang language
> > support. How can I get more information about what is going on with
> > AT_CHECK_ENV?
>
> AT_CHECK_ENV is defined in tests/local.at; it's job is to exempt a number
> of environment variables that don't fit autoconf namespace rules but do
> get set by autoconf.  My guess is that the list just needs to be made
> larger; you should be able to inspect the testsuite directory after a
> failed tests to compare clean-state-env.before and clean-state-env.after
> to see what environment variables are being set by Erlang support (and
> whether we should rename them into the autoconf namespace or exempt them).

OK. I will have to spend more time to check that.
I attached a new patch which takes into account your other comments below.

> > Also, those tests will fail if Erlang is not installed. I could use
> > AT_SKIP_IF for each test group to don't perform a test group, but it's
> > tedious. Is there an equivalent to AT_SKIP_IF that works for a whole
> > test group category (the whole erlang.at file)?
>
> Yes - the trick is to write a wrapper macro:
>
> m4_define([AT_CHECK_ERLANG],
> [AT_KEYWORDS([Erlang])
> AT_SKIP_IF([<erlang not found>])
> AT_CHECK($@)])

OK. Done.

> then use AT_CHECK_ERLANG through the rest of erlang.at.  I can't apply the
> patch until we cater to the fact that Erlang is not yet universal.
>
> > +   Add autotests for Erlang macros.
> > +   * tests/erlang.at: Added tests for all macros in erlang.m4.
> > +   * tests/Makefile.am: Added erlang.at.
> > +   * tests/suite.at: Likewise.
> > +   * tests/compile.at: Added test for extension of Erlang files.
>
> Given the regressions that Erlang has had in the past, I'd like to include
> this in 2.64, but my time schedule is that I either release it by this
> Saturday, or it waits till mid-August after my vacation.

Anyway the main purpose of autotests is to prevent regressions?
My patch does not add any feature, and I have not found any bug in the latest 
version of the repository.
So it may be OK to add the tests after releasing?

> > +** The following macros are now covered by autotests:
> > +   AC_ERLANG_CHECK_LIB, AC_ERLANG_SUBST_ROOT_DIR,
> > +   AC_ERLANG_SUBST_LIB_DIR, AC_ERLANG_SUBST_INSTALL_LIB_DIR,
> > +   AC_ERLANG_SUBST_ERTS_VER, and the macros for tests in Erlang
>
> How about we just condense this to:
>
> The autoconf testsuite now validates all Erlang macros.

OK. Done.

> > +++ b/tests/compile.at
> > @@ -43,9 +43,13 @@ AC_LANG_PUSH(C++)
> >  # C++ C C
> >  AC_LANG(C++)
> >  # C++ C C
> > +AC_LANG_PUSH(Erlang)
> > +# Erlang C++ C C
>
> Am I correct that this test has no problems regardless of whether Erlang
> is present?

Yes, it's OK.

> > +++ b/tests/erlang.at
> > @@ -0,0 +1,178 @@
> > +#                                                  -*- Autotest -*-
> > +
> > +AT_BANNER([Erlang low level compiling and utility macros.])
> > +
> > +# Copyright (C) 2000, 2001, 2003, 2008 Free Software Foundation, Inc.
>
> Fix the copyright years - it is a new file, and needs only 2009.

OK. Done.

> > +AT_SETUP([Erlang])
> > +
> > +AT_DATA([configure.ac],
> > +[[AC_INIT
> > +AC_ERLANG_NEED_ERL
> > +AC_ERLANG_NEED_ERLC
> > +AC_LANG(Erlang)
>
> Let's get in the habit of following our documentation: AC_LANG([Erlang])

OK. Done in erlang.at and also in compile.at for other cases I have found.

Thanks,
--
Romain Lenglet

autoconf_erlang_autotests_2009-07-21.diff
Description: Text Data