Hello automakers. Resuming an old thread ...
Reference:
<http://lists.gnu.org/archive/html/automake-patches/2011-01/msg00192.html>
On Monday 17 January 2011, Ralf Wildenhues wrote:
> [ Cc:ing Jim because of ideas at the end ]
>
And CC:ing him again because I'd like an ACK from him (see below).
> * Stefano Lattarini wrote on Sun, Jan 16, 2011 at 03:56:07PM CET:
> > This patch stemmed from this discussion:
> > <http://lists.gnu.org/archive/html/automake-patches/2011-01/msg00144.html>
> >
> > I'd like to have the patch applied to maint, to make eventual integration
> > of new tests easier. But the follow-up patches converting the testsuite
> > to the use of skip_() and providing more informative messages for skipped
> > tests should go to master only, to avoid unecessary "merging churn".
> >
> > OK?
>
> Unfortunately not, for a couple of reasons. I'm sorry I didn't think
> about it in more depth before, but there are several things that I think
> could be better with this patch.
>
> First off, a legal one, quoting maintain.info:
>
> When you copy legally significant code from another free software
> package with a GPL-compatible license, you should look in the package's
> records to find out the authors of the part you are copying, and list
> them as the contributors of the code that you copied. If all you did
> was copy it, not write it, then for copyright purposes you are _not_
> one of the contributors of _this_ code.
>
> So, if we copy the code, then Jim is the author of it.
>
OK, I've added Jim as the primary author of this patch (both in the
ChangeLog and with "git --author"). I'll wait his ACK on this attribution
before pushing.
> Then, there are issues that the code exposes that I think we should
> address:
>
> > tests: new subroutines for test skipping/failing.
> >
> > * tests/defs.in (Exit): Move definition of this function earlier.
> > (warn_, skip_, fail_, framework_failure_): New functions, inspired
> > to the homonyms in gnulib's tests/init.sh.
>
> > ($stderr_fileno_): New global variable, used by the new functions
> > above.
> > * tests/README: Updated.
>
> > --- a/tests/defs.in
> > +++ b/tests/defs.in
>
> > @@ -88,6 +88,31 @@ echo "$PATH"
> > # (See note about `export' in the Autoconf manual.)
> > export PATH
> >
> > +# Print warnings (e.g., about skipped and failed tests) to this file
> > number.
> > +# Override by putting, say:
> > +# stderr_fileno_=9; export stderr_fileno_; exec 9>&2; $(SHELL)
> > +# in the definition of TESTS_ENVIRONMENT.
>
> That may be good advice in the context of gnulib. However, it describes
> what is essentially a bug, or at least usability issue, in Automake:
> that the test author cannot write to the original stderr with the
> parallel-tests driver any more, and has to use a workaround which breaks
> user overrides of TESTS_ENVIRONMENT.
>
> I think this should be addressed in the driver(s) ...
>
> [CUT]
>
> I further wonder whether we should finally introduce $(AM_TESTS_ENVIRONMENT)
> and reserve the non-AM_ variable for environment settings for the user.
>
> What do you think?
>
JFTR: we agreed on this, and the change went in (into master) with the
commits `v1.11-349-g12f48fa' and `v1.11-348-g95bbdf1' (with some more
churn than expected, due to some excessive haste on my part).
> > +# This is useful when using automake's parallel tests mode, to print
> > +# the reason for skip/failure to console, rather than to the .log files.
> > +: ${stderr_fileno_=2}
> > +
> > +warn_() { echo "$@" 1>&$stderr_fileno_; }
> > +fail_() { warn_ "$me: failed test: $@"; Exit 1; }
> > +skip_() { warn_ "$me: skipped test: $@"; Exit 77; }
> > +framework_failure_() { warn_ "$me: set-up failure: $@"; Exit 99; }
>
> space before ()
>
Done (both here and in the gnulib repository).
The updated patch is attached; when I got Jim ACK, I'll push it to maint,
to make eventual integration of new testcases using the new subroutines
easier (and because the patch is really non-invasive). I'll follow-up
soonish with patches converting the testsuite to the use of skip_() and
of more informative skip messages; this patches will be applied to a new
temporary public branch "testsuite-work", meant to be finally merged into
master (once it's stabilized).
Regards,
Stefano
From 2c8b68396a48f70de2856f0747968225f4906abf Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Sun, 16 Jan 2011 15:36:07 +0100
Subject: [PATCH] tests: new subroutines for test skipping/failing.
* tests/defs.in (Exit): Move definition of this function earlier.
(warn_, skip_, fail_, framework_failure_): New functions, inspired
to the homonyms in gnulib's tests/init.sh.
($stderr_fileno_): New global variable, used by the new functions
above.
* tests/README: Updated.
From a suggestion by Ralf Wildenhues.
---
ChangeLog | 12 ++++++++++++
tests/README | 5 +++++
tests/defs.in | 38 ++++++++++++++++++++++++++------------
3 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4417d9b..3887852 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2011-04-23 Jim Meyering <meyer...@redhat.com>
+ Stefano Lattarini <stefano.lattar...@gmail.com>
+
+ test defs: new subroutines for test skipping/failing
+ * tests/defs.in (Exit): Move definition of this function earlier.
+ (warn_, skip_, fail_, framework_failure_): New functions, inspired
+ to the homonyms in gnulib's tests/init.sh.
+ ($stderr_fileno_): New global variable, used by the new functions
+ above.
+ * tests/README: Updated.
+ From a suggestion by Ralf Wildenhues.
+
2011-04-22 Stefano Lattarini <stefano.lattar...@gmail.com>
testsuite: more environment sanitization
diff --git a/tests/README b/tests/README
index 93f9cbf..26ce3ff 100644
--- a/tests/README
+++ b/tests/README
@@ -100,6 +100,11 @@ Do
Include ./defs in every test script (see existing tests for examples
of how to do this).
+ Use the `skip_' function to skip tests, with a meaningful message if
+ possible. Where convenient, use the `warn_' function to print generic
+ warnings, and the `fail_' function for test failures. Finally, you may
+ use the `framework_fail_' function for hard errors.
+
For tests that use the `parallel-tests' Automake option, set the shell
variable `parallel_tests' to "yes" before including ./defs. Also,
use for them a name that ends in `-p.test' and does not clash with any
diff --git a/tests/defs.in b/tests/defs.in
index ef35d9f..4e0db16 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -122,6 +122,32 @@ echo "$PATH"
# (See note about `export' in the Autoconf manual.)
export PATH
+# We use a trap below for cleanup. This requires us to go through
+# hoops to get the right exit status transported through the signal.
+# So use `Exit STATUS' instead of `exit STATUS' inside of the tests.
+# Turn off errexit here so that we don't trip the bug with OSF1/Tru64
+# sh inside this function.
+Exit ()
+{
+ set +e
+ (exit $1)
+ exit $1
+}
+
+# Print warnings (e.g., about skipped and failed tests) to this file
+# number. Override by putting, say:
+# stderr_fileno_=9; export stderr_fileno_; exec 9>&2;
+# in the definition of AM_TESTS_ENVIRONMENT.
+# This is useful when using automake's parallel tests mode, to print the
+# reason for skip/failure to console, rather than to the *.log files.
+: ${stderr_fileno_=2}
+
+# Copied from Gnulib's `tests/init.sh'.
+warn_ () { echo "$@" 1>&$stderr_fileno_; }
+fail_ () { warn_ "$me: failed test: $@"; Exit 1; }
+skip_ () { warn_ "$me: skipped test: $@"; Exit 77; }
+framework_failure_ () { warn_ "$me: set-up failure: $@"; Exit 99; }
+
for tool in : $required
do
# Check that each required tool is present.
@@ -278,18 +304,6 @@ case "$srcdir" in
;;
esac
-# We use a trap below for cleanup. This requires us to go through
-# hoops to get the right exit status transported through the signal.
-# So use `Exit STATUS' instead of `exit STATUS' inside of the tests.
-# Turn off errexit here so that we don't trip the bug with OSF1/Tru64
-# sh inside this function.
-Exit ()
-{
- set +e
- (exit $1)
- exit $1
-}
-
curdir=`pwd`
testSubDir=$me.dir
test ! -d $testSubDir || {
--
1.7.2.3
