Re: [PATCH] {maint} tests: new subroutines for test skipping/failing

[Stefano Lattarini]

On Friday 04 February 2011, Ralf Wildenhues wrote:
> Hello Stefano,
>
Hi Ralf, and sorry for the delay.

> you've already applied this patch, but I have a couple of nits anyway:
> 
> * Stefano Lattarini wrote on Mon, Jan 24, 2011 at 04:04:30PM CET:
> > Subject: [PATCH] coverage: more tests on simple and parallel test drivers
> > 
> > * tests/parallel-tests-subdir.test: New test.
> > * tests/check-exported-srcdir.test: Likewise.
> > * tests/check-tests-in-builddir.test: Likewise.
> > * tests/check-tests_environment.test: Likewise.
> > * tests/Makefile.am (TESTS): Update.
> 
> > --- /dev/null
> > +++ b/tests/check-exported-srcdir.test
> 
> > +# Check that the "Simple Tests" driver (either with or without the
> 
> Since this test source will be used for both drivers, why not
>   s/"Simple Tests"/testsuite/
>
OK.

> > +# parallel-tests option enabled) exports the `srcdir' value in the
> > +# environment of the tests.  This is documented in the manual.
> 
> > +mkdir SrcDir BuildDir
> 
> Please let's avoid CamelCase in the future.  We've managed almost two
> decades without it, we don't need it now.  (Plus, it doesn't fit the
> GCS style.)
>
OK.  Should I also change the test to avoid it?  (From your formulation
above I'd guess the answer is "no", but better be sure).


> > --- /dev/null
> > +++ b/tests/check-tests-in-builddir.test
> 
> > +# Check that the "Simple Tests" driver can find test in the srcdir as
> > +# well as in builddir, and that is prefers those in the builddir.
> 
> See above.
>
Fixed.

> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat >> configure.in << 'END'
> > +AC_OUTPUT
> > +END
> > +
> > +cat > Makefile.am << 'END'
> > +TESTS = foo.test bar.test
> > +EXTRA_DIST = $(TESTS)
> > +END
> > +
> > +cat > foo.test << 'END'
> > +#! /bin/sh
> > +exit ${FOO_EXIT_STATUS-0}
> > +END
> > +chmod a+x foo.test
> > +
> > +$ACLOCAL
> > +$AUTOCONF
> > +$AUTOMAKE -a
> > +
> > +mkdir build
> > +cd build
> > +
> > +../configure
> > +
> > +cat > bar.test << 'END'
> > +#! /bin/sh
> > +exit 0
> > +END
> > +chmod a+x bar.test
> > +
> 
> unset FOO_EXIT_STATUS || :
>
I hadn't thought about this because it seemed really unlikely that the
user could have a variable named `FOO_EXIT_STATUS' in his environment.
But some extra safety won't hurt, I guess.

> > +$MAKE check >out 2>&1 || { cat out; Exit1; }
> > +cat out
> > +grep '\.\./foo' out && Exit 1
> > +grep '^PASS: foo.test *$' out
> > +grep '^PASS: bar.test *$' out
> > +
> > +rm -f test-suite.log foo.log bar.log
> > +
> > +FOO_EXIT_STATUS=1 $MAKE check >out 2>&1 && { cat out; Exit1; }
> > +cat out
> > +grep '\.\./foo' out && Exit 1
> > +grep '^FAIL: foo.test *$' out
> > +grep '^PASS: bar.test *$' out
> > +
> > +rm -f test-suite.log foo.log bar.log
> > +
> > +# Check that if the same test is present in srcdir and builddir,
> > +# the one in builddir is preferred.
> > +cp bar.test foo.test
> > +FOO_EXIT_STATUS=1 $MAKE check >out 2>&1 || { cat out; Exit1; }
> > +cat out
> > +grep '^PASS: foo.test *$' out
> > +grep '^PASS: bar.test *$' out
> > +
> > +# The tests in the builddir must be preferred also by "make dist".
> > +FOO_EXIT_STATUS=1 $MAKE distcheck


> > --- /dev/null
> > +++ b/tests/check-tests_environment.test
> 
> > +# "Simple Tests" testsuite driver: check TESTS_ENVIRONMENT support.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat >> configure.in << 'END'
> > +AC_OUTPUT
> > +END
> > +
> > +cat > Makefile.am << 'END'
> > +TESTS = foo.test
> > +EXTRA_DIST = $(TESTS)
> > +END
> > +
> > +cat > foo.test << 'END'
> > +#! /bin/sh
> > +test x"$FOO" = x"ok"
> > +END
> > +chmod a+x foo.test
> > +
> > +$ACLOCAL
> > +$AUTOCONF
> > +$AUTOMAKE -a
> > +
> > +./configure
> > +
> > +FOO=bad TESTS_ENVIRONMENT='FOO=ok'  $MAKE check
> > +FOO=ok  TESTS_ENVIRONMENT='FOO=bad' $MAKE check && Exit 1
> 
> I wonder whether we need to unset TESTS_ENVIRONMENT in tests/defs.
>
That seems a good idea.  Will do in a follow-up patch.

> > --- /dev/null
> > +++ b/tests/parallel-tests-subdir.test
> 
> > +# Check that the parallel-tests driver creates parent directories for
> > +# the log files when needed.
> 
> I think we have a test for this already.  Just
>   grep 'TESTS.*/' tests/*.test
>
Oops, that's true, it's already checked in `check8.test' (albeit
indirectly, so I missed that).  Well, at least no existing test was
checking for a test in a sub-subdir like this one does, so it hasn't
been a completely useless addition after all.

> (I can understand your desire for more descriptive test names, but not
> in easy cases like this one.  No matter how descriptive, you will still
> need to search existing tests now and then, simply because the naming
> may not be unique.)
> 
> > +parallel_tests=yes
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat >> configure.in << 'END'
> > +AC_OUTPUT
> > +END
> > +
> > +cat > Makefile.am << 'END'
> > +TESTS = dir1/foo.test dir2/dir3/foo.test
> > +TEST_LOG_COMPILER = sh
> > +END
> > +
> > +mkdir dir1 dir2 dir2/dir3
> > +echo : > dir1/foo.test
> > +echo : > dir2/dir3/foo.test
> > +
> > +$ACLOCAL
> > +$AUTOCONF
> > +$AUTOMAKE -a
> > +
> > +mkdir build
> > +cd build
> > +../configure
> > +$MAKE check
> > +find . # For debugging.
> > +test -f test-suite.log
> > +test -f dir1/foo.log
> > +test -f dir2/dir3/foo.log
> 
> Thanks,
> Ralf
> 
Attached is what I'll push (in 72 hours) if there are no further
objections.

Regards,
  Stefano

From 5ed043ad03d36be44500501ec0d69f3ba736911d Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Sun, 6 Feb 2011 19:43:22 +0100
Subject: [PATCH] tests: tweak few tests on simple and parallel test drivers

* tests/check-exported-srcdir.test: Improve heading comments.
* tests/check-tests-in-builddir.test: Likewise.  Also, unset the
`FOO_EXIT_STATUS' variable, so that any pre-existing value in the
environment won't risk to interfere with the test.

Suggestions by Ralf Wildenhues.
---
 ChangeLog                          |    9 +++++++++
 tests/check-exported-srcdir.test   |    2 +-
 tests/check-tests-in-builddir.test |    4 +++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 31bdd66..3750915 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-02-06  Stefano Lattarini  <address@hidden>
+
+       tests: tweak few tests on simple and parallel test drivers
+       * tests/check-exported-srcdir.test: Improve heading comments.
+       * tests/check-tests-in-builddir.test: Likewise.  Also, unset the
+       `FOO_EXIT_STATUS' variable, so that any pre-existing value in the
+       environment won't risk to interfere with the test.
+       Suggestions by Ralf Wildenhues.
+
 2011-02-01  Stefano Lattarini  <address@hidden>
 
        coverage: more tests on simple and parallel test drivers
diff --git a/tests/check-exported-srcdir.test b/tests/check-exported-srcdir.test
index 9209fc8..dc02b22 100755
--- a/tests/check-exported-srcdir.test
+++ b/tests/check-exported-srcdir.test
@@ -14,7 +14,7 @@
 # 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 that the "Simple Tests" driver (either with or without the
+# Check that the testsuite driver (either with or without the
 # parallel-tests option enabled) exports the `srcdir' value in the
 # environment of the tests.  This is documented in the manual.
 
diff --git a/tests/check-tests-in-builddir.test 
b/tests/check-tests-in-builddir.test
index b30999b..2d0e423 100755
--- a/tests/check-tests-in-builddir.test
+++ b/tests/check-tests-in-builddir.test
@@ -14,7 +14,7 @@
 # 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 that the "Simple Tests" driver can find test in the srcdir as
+# Check that the testsuite driver can find test in the srcdir as
 # well as in builddir, and that is prefers those in the builddir.
 
 . ./defs || Exit 1
@@ -36,6 +36,8 @@ exit ${FOO_EXIT_STATUS-0}
 END
 chmod a+x foo.test
 
+unset FOO_EXIT_STATUS || :
+
 $ACLOCAL
 $AUTOCONF
 $AUTOMAKE -a
-- 
1.7.2.3