bug#7995: [PATCH 2/4] Testsuite coverage for Guile support.

[Stefano Lattarini]

Hi Ralf.

Knowing almost nothing about guile, I'm not able to help in the
discussion of design/implementation, but I'll throw in my two cents
w.r.t. the testsuite additions ...

On Sunday 20 February 2011, Ralf Wildenhues wrote:
> * tests/guile1.test, tests/guile2.test, tests/guile3.test,
> tests/guile4.test: New tests.
> * tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
> ---
>  ChangeLog         |    5 ++
>  tests/Makefile.am |    8 +++
>  tests/Makefile.in |    7 ++-
>  tests/guile1.test |   54 ++++++++++++++++++++++
>  tests/guile2.test |  132 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/guile3.test |   91 ++++++++++++++++++++++++++++++++++++
>  tests/guile4.test |   48 +++++++++++++++++++
>
Couldn't we use more expressive/clean names for the tests?
Suggestions below.

>  7 files changed, 344 insertions(+), 1 deletions(-)
>  create mode 100755 tests/guile1.test
>  create mode 100755 tests/guile2.test
>  create mode 100755 tests/guile3.test
>  create mode 100755 tests/guile4.test
> 
> diff --git a/ChangeLog b/ChangeLog
> index c338e8a..5483ce4 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,10 @@
>  2011-02-20  Ralf Wildenhues  <address@hidden>
>  
> +     Testsuite coverage for Guile support.
> +     * tests/guile1.test, tests/guile2.test, tests/guile3.test,
> +     tests/guile4.test: New tests.
> +     * tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
> +
>       Documentation for Guile support.
>       * doc/automake.texi (Guile): New chapter.
>       (Top, Other GNU Tools): Update menus.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index e3eb9e9..b993678 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -24,6 +24,10 @@ all.test \
>  auxdir2.test \
>  cond17.test \
>  gcj6.test \
> +guile1.test \
> +guile2.test \
> +guile3.test \
> +guile4.test \
>  override-conditional-2.test \
>  txinfo5.test
>  
> @@ -460,6 +464,10 @@ gnuwarn2.test \
>  gnits.test \
>  gnits2.test \
>  gnits3.test \
> +guile1.test \
> +guile2.test \
> +guile3.test \
> +guile4.test \
>  header.test \
>  help.test \
>  help2.test \
> diff --git a/tests/Makefile.in b/tests/Makefile.in
> index 1f367cd..e449137 100644
> --- a/tests/Makefile.in
> +++ b/tests/Makefile.in
> @@ -284,7 +284,8 @@ top_srcdir = @top_srcdir@
>  MAINTAINERCLEANFILES = $(parallel_tests) $(instspc_tests)
>  EXTRA_DIST = ChangeLog-old gen-parallel-tests instspc-tests.sh \
>       $(TESTS)
> -XFAIL_TESTS = all.test auxdir2.test cond17.test gcj6.test \
> +XFAIL_TESTS = all.test auxdir2.test cond17.test gcj6.test guile1.test \
> +     guile2.test guile3.test guile4.test \
>       override-conditional-2.test txinfo5.test \
>       $(instspc_xfail_tests)
>  parallel_tests = backcompat5-p.test check-exported-srcdir-p.test \
> @@ -722,6 +723,10 @@ gnuwarn2.test \
>  gnits.test \
>  gnits2.test \
>  gnits3.test \
> +guile1.test \
> +guile2.test \
> +guile3.test \
> +guile4.test \
>  header.test \
>  help.test \
>  help2.test \


> diff --git a/tests/guile1.test b/tests/guile1.test
> new file mode 100755
> index 0000000..94efa82
> --- /dev/null
> +++ b/tests/guile1.test
> @@ -0,0 +1,54 @@
> +#! /bin/sh
> +# Copyright (C) 2011 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2, or (at your option)
> +# any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Basic Guile support: guile_GUILE.
> +
What about calling this test `guile-basic.test'?

> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >>configure.in <<\END
> +AM_PATH_GUILE
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am <<\END
> +guile_GUILE = foo.scm bar.scm baz.scm
> +END
> +
> +: >foo.scm
> +: >bar.scm
> +: >baz.scm
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE --add-missing
> +
> +grep '^GUILECOMPILE' Makefile.in
>
grep '^GUILECOMPILE *=' ?

> +grep '^GUILEFLAGS' Makefile.in
> +
Similarly.

> +inst=`pwd`/inst
> +./configure --prefix="$inst"
> +
> +env GUILE_TOOLS=false $MAKE -e && Exit 1
> +env AM_GUILEFLAGS=--nosuchflag $MAKE -e && Exit 1
> +env GUILEFLAGS=--nosuchflag $MAKE -e && Exit 1
> +
What if guile-tools is not installed, and `GUILE_TOOLS' gets
defined to `:'?  Won't the commands above succed in this case?

Also, maybe we could ensure also that really GUILEFLAGS really takes
precedence over AM_GUILEFLAGS, and that GUILE_TOOLS can be successfully
overridden by a "fake script", similarly to what is done by tests
`ylflags.test' and `lflags.test'.  That might be done in another test
`guile-flags.test' maybe.

> +$MAKE
> +$MAKE install
> +$MAKE distcheck
> +
> +:


> diff --git a/tests/guile2.test b/tests/guile2.test
> new file mode 100755
> index 0000000..d355518
> --- /dev/null
> +++ b/tests/guile2.test
> @@ -0,0 +1,132 @@
> +#! /bin/sh
> +# Copyright (C) 2011 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2, or (at your option)
> +# any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Ensure that we compile and install Guile sources properly.
> +
What about calling this test `guile-inst-and-dist.test'?

> +required='guile-tools'
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >>configure.in <<\END
> +AM_PATH_GUILE([1.8],,
> +  [AC_MSG_ERROR([Guile installation missing or too old for compilation],
> +             [77])])
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am <<\END
> +guile_GUILE = compiled-and-installed.scm
> +nodist_guile_GUILE = not-distributed.scm
> +guile_DATA = not-compiled.scm
> +noinst_GUILE = compiled-but-not-installed.scm
> +dist_noinst_DATA = just-distributed-blob.scm
> +nobase_guile_GUILE = installed-in-subdir/foo.scm
> +pkgguile_GUILE = installed-in-package-dir.scm
> +
> +not-distributed.scm:
> +     : >$@
> +
> +CLEANFILES = not-distributed.scm
> +
> +check-dist:
> +     test -f $(distdir)/compiled-and-installed.scm
> +     test ! -f $(distdir)/not-distributed.scm
> +     test -f $(distdir)/not-compiled.scm
> +     test -f $(distdir)/compiled-but-not-installed.scm
> +     test -f $(distdir)/just-distributed-blob.scm
> +     test -f $(distdir)/installed-in-subdir/foo.scm
> +     test -f $(distdir)/installed-in-package-dir.scm
> +     test -z "`find $(distdir) -name \*.go`"
> +
I'd suggest to use:
        find $(distdir) -name \*.go | grep . && exit 1; :
here, so that in case of failure we obtain the names/paths of the
unexpected *.go files.

> +END
> +
> +mkdir installed-in-subdir
> +
> +: >compiled-and-installed.scm
> +: >not-compiled.scm
> +: >compiled-but-not-installed.scm
> +: >just-distributed-blob.scm
> +: >installed-in-subdir/foo.scm
> +: >installed-in-package-dir.scm
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE --add-missing
> +
> +inst=`pwd`/inst
> +./configure --prefix="$inst"
> +$MAKE
> +test -f compiled-and-installed.go
> +test -f not-distributed.go
> +test ! -f not-compiled.go
> +test -f compiled-but-not-installed.go
> +test ! -f just-distributed-blob.go
> +test -f installed-in-subdir/foo.go
> +test -f installed-in-package-dir.go
> +
> +guile_version=
> +if : new install layout; then
> +  guiledir=$inst/share/guile/site/$guile_version
> +  guileexecdir=$inst/lib/guile/$guile_version/ccache
> +else
> +  guiledir=$inst/share/guile/site
> +  guileexecdir=$inst/lib
> +fi
> +
> +$MAKE install-data
> +test -f "$guiledir/compiled-and-installed.scm"
> +test -f "$guiledir/not-distributed.scm"
> +test ! -f "$guiledir/not-compiled.scm"
> +test ! -f "$guiledir/compiled-but-not-installed.scm"
> +test ! -f "$guiledir/just-distributed-blob.scm"
> +test -f "$guiledir/installed-in-subdir/foo.scm"
> +test -f "$guiledir/$me/installed-in-package-dir.scm"
> +
> +installed_go=`find "$inst" -name \*.go`
> +test -z "$installed_go"
>
Bikeshedding here, but I'd substitue this two lines with:
  find "$inst" -name \*.go | grep . && Exit 1

> +$MAKE uninstall
> +
> +$MAKE install-exec
> +if : can compile; then
> +  test -f "$guileexecdir/compiled-and-installed.go"
> +  test -f "$guileexecdir/not-distributed.go"
> +  test ! -f "$guileexecdir/not-compiled.go"
> +  test ! -f "$guileexecdir/compiled-but-not-installed.go"
> +  test ! -f "$guileexecdir/just-distributed-blob.go"
> +  test -f "$guileexecdir/installed-in-subdir/foo.go"
> +  test -f "$guileexecdir/$me/installed-in-package-dir.go"
> +  installed_scm=`find "$inst" -name \*.scm`
> +  test -z "$installed_scm"
> +else
> +  installed_go=`find "$inst" -name \*.go`
> +  test -z "$installed_go"
>
See above.

> +fi
> +
> +$MAKE clean
> +
> +test ! -f compiled-and-installed.go
> +test ! -f not-distributed.go
> +test ! -f not-compiled.go
> +test ! -f compiled-but-not-installed.go
> +test ! -f just-distributed-blob.go
> +test ! -f installed-in-subdir/foo.go
> +test ! -f installed-in-package-dir.go
> +
> +$MAKE check-dist
> +
> +$MAKE distcheck
> +
> +:


> diff --git a/tests/guile3.test b/tests/guile3.test
> new file mode 100755
> index 0000000..4be3c82
> --- /dev/null
> +++ b/tests/guile3.test
> @@ -0,0 +1,91 @@
> +#! /bin/sh
> +# Copyright (C) 2011 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2, or (at your option)
> +# any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Check for Guile compiled extensions.
> +
What about calling this test `guile-compiled-ext.test'?

> +required='guile-tools libtoolize'
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >>configure.in <<\END
> +AC_PROG_CC
> +AC_PROG_LIBTOOL
> +AM_PATH_GUILE
> +AC_PATH_PROG([GUILE], [guile], [exit 77])
> +: "${GUILECPPFLAGS=`guile-config compile`}"
> +: "${GUILECFLAGS=`guile-config compile`}"
> +: "${GUILELIBS=`guile-config link`}"
> +AC_SUBST([GUILECPPFLAGS])
> +AC_SUBST([GUILECFLAGS])
> +AC_SUBST([GUILELIBS])
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am <<\END
> +guileext_LTLIBRARIES = foo.la
> +foo_la_SOURCES = foo.c
> +foo_la_LDFLAGS = -module -avoid-version
> +foo_la_LIBADD = $(GUILELIBS)
> +AM_CFLAGS = $(GUILECFLAGS)
> +AM_CPPFLAGS = $(GUILECPPFLAGS)
> +
> +guile_GUILE = load-foo.scm
> +
> +# Try out the uninstalled module.
> +check-local:
> +     $(LIBTOOL) --mode=execute -dlopen foo.la $(GUILE) load-foo.scm
> +# Try out the installed module.
> +installcheck-local:
> +     $(GUILE) load-foo.scm
> +END
> +
> +cat > load-foo.scm <<\END
> +(dynamic-call "init_the_answer" (dynamic-link "foo"))
> +(answer)
> +END
> +
> +cat >foo.c <<\END
> +#include <libguile.h>
> +
> +SCM
> +the_answer (void)
> +{
> +  return scm_int2num (42);
> +}
> +
> +void
> +init_the_answer (void)
> +{
> +  scm_c_define_gsubr ("answer", 0, 0, 0, the_answer);
> +}
> +END
> +
> +inst=`pwd`/inst
> +
> +libtoolize --force
> +$ACLOCAL
> +$AUTOMAKE --add-missing
> +$AUTOCONF
> +./configure --prefix="$inst"
> +$MAKE
> +$MAKE check
> +$MAKE install
> +$MAKE clean
> +$MAKE installcheck
> +$MAKE distcheck
> +
> +:


> diff --git a/tests/guile4.test b/tests/guile4.test
> new file mode 100755
> index 0000000..47a0ad9
> --- /dev/null
> +++ b/tests/guile4.test
> @@ -0,0 +1,48 @@
> +#! /bin/sh
> +# Copyright (C) 2011 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2, or (at your option)
> +# any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Per-target flags should not cause compiled Guile sources to be renamed.
> +
What about calling this test `guile-per-target-flags.test'?  Or if that
seems too long, `guile-spcflg.test'? 

> +required=guile-tools
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >>configure.in <<\END
> +AM_PATH_GUILE
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am <<\END
> +guile_GUILE = foo.scm
> +foo_scm_GUILEFLAGS = ...
> +END
> +
> +: >foo.scm
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE --add-missing
> +
> +inst=`pwd`/inst
> +./configure --prefix="$inst"
> +$MAKE
> +test -f foo.go
> +test ! -f foo_go_foo.go
> +
IMHO you could err on the side of caution here, and tighten this to:

  test -f foo.go
  ls *.go | grep -v '^foo\.go$' | grep . && Exit 1

or even:

  test -f foo.go
  find . -name \*.go | grep -v '^\./foo\.go$' | grep . && Exit 1

> +$MAKE distcheck
> +
> +:
> 

Also, IMHO, some deeper tests about the code in m4/guile.m4 would be
required (similarly for what is done for python).

Regards,
     Stefano