Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment

[Stefano Lattarini]

On Monday 18 April 2011, Stefano Lattarini wrote:
> On Monday 18 April 2011, Stefano Lattarini wrote:
> > On Monday 18 April 2011, Ralf Wildenhues wrote:
> > > | > am_test_name is better, but doesn't explain either why it would be
> > > | > needed in the first place.
> > > | >
> > > | Second patch of:
> > > |  
> > > <http://lists.gnu.org/archive/html/automake-patches/2011-02/msg00044.html>
> > > | And possible similar patches in the future.
> > > 
> > > That still doesn't answer why a user-available override would be needed.
> > >
> > In truth, the variable is not meant to be user-overridable.  That's just
> > an accident (i.e., I couldn't think of a better impelementation that would
> > disallow this).
> > 
> > > It answers why you want to make $me overridable *from within* the
> > > testsuite, but not why for the user running 'make check'.
> > >
> > In fact, I don't want to.  That would probably just cause random breakage.
> > 
> > > For example, tests/Makefile.am could have
> > >   unset me ||:;
> > > 
> > > as part of AM_TESTS_ENVIRONMENT.  That wouldn't defeat users running
> > > tests themselves, but it would defeat breakage induced by differences
> > > in the user environment.
> > >
> > I like your solution more than mine; I withdraw my patch, and I'll soon
> > write a new one on the lines you've suggested (that is, once support for
> > AM_TESTS_ENVIRONMENT is in place)?
> > 
> > Thanks, and sorry for the noise,
> >   Stefano
> > 
> Done in the attached patch, using TESTS_ENVIRONMENT for the moment.  We can
> switch to AM_TESTS_ENVIRONMENT once it's in place, OK?
> 
> I'll push by tomorrow evening if there is no objection.
> 
> Regards,
>   Stefano
> 
I've merged the patch to maint and master now, and also applied the attached
follow-up patches to master.

Regards,
  Stefano

From 986c68e702f191e05b09e580e6170502b4acf65a Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Tue, 19 Apr 2011 22:52:54 +0200
Subject: [PATCH 1/2] tests: prefer AM_TESTS_ENVIRONMENT to TESTS_ENVIRONMENT

* tests/Makefile.am (TESTS_ENVIRONMENT): Renamed to ...
(AM_TESTS_ENVIRONMENT): ... this.
---
 ChangeLog         |    6 ++++++
 tests/Makefile.am |    2 +-
 tests/Makefile.in |    2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4e4c220..6ea78a2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-04-19  Stefano Lattarini  <address@hidden>
+
+       tests: prefer AM_TESTS_ENVIRONMENT to TESTS_ENVIRONMENT
+       * tests/Makefile.am (TESTS_ENVIRONMENT): Renamed to ...
+       (AM_TESTS_ENVIRONMENT): ... this.
+
 2011-04-18  Stefano Lattarini  <address@hidden>
 
        tests: don't allow `$me' to be overridden from the environment
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b95d54c..1c1c200 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -100,7 +100,7 @@ XFAIL_TESTS += $(instspc_xfail_tests)
 
 # The testsuite variable `$me' should be overridable from the
 # test scripts, but not from the environment.
-TESTS_ENVIRONMENT = test x"$$me" = x || unset me;
+AM_TESTS_ENVIRONMENT = test x"$$me" = x || unset me;
 
 TESTS = \
 aclocal.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 9beb968..76e4588 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -365,7 +365,7 @@ instspc_xfail_tests = instspc-squote-build.test \
 
 # The testsuite variable `$me' should be overridable from the
 # test scripts, but not from the environment.
-TESTS_ENVIRONMENT = test x"$$me" = x || unset me;
+AM_TESTS_ENVIRONMENT = test x"$$me" = x || unset me;
 TESTS = \
 aclocal.test \
 aclocal3.test \
-- 
1.7.2.3

From 942379cc3862ca20003ccea44edff90e6a13b314 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Tue, 19 Apr 2011 22:57:39 +0200
Subject: [PATCH 2/2] tests: in self-checks, use $SHELL, not /bin/sh

* tests/self-check-me-in-env.test: Include `./defs-static' to get
the definition of `$SHELL'.  Use `$SHELL' instead of `/bin/sh' to
execute the tests.
---
 ChangeLog                       |    7 +++++++
 tests/self-check-me-in-env.test |    6 ++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6ea78a2..6aeafd5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2011-04-19  Stefano Lattarini  <address@hidden>
 
+       tests: in self-checks, use $SHELL, not /bin/sh
+       * tests/self-check-me-in-env.test: Include `./defs-static' to get
+       the definition of `$SHELL'.  Use `$SHELL' instead of `/bin/sh' to
+       execute the tests.
+
+2011-04-19  Stefano Lattarini  <address@hidden>
+
        tests: prefer AM_TESTS_ENVIRONMENT to TESTS_ENVIRONMENT
        * tests/Makefile.am (TESTS_ENVIRONMENT): Renamed to ...
        (AM_TESTS_ENVIRONMENT): ... this.
diff --git a/tests/self-check-me-in-env.test b/tests/self-check-me-in-env.test
index 344a861..966088d 100755
--- a/tests/self-check-me-in-env.test
+++ b/tests/self-check-me-in-env.test
@@ -18,11 +18,13 @@
 # Make sure that the testsuite initialization code complains when
 # $me is set in the environment.
 
+. ./defs-static || exit 1
+
 set -x
 
 exec 5>&1
-env me=foo /bin/sh -c '. ./defs' foo.test && exit 1
-env me=foo /bin/sh -c '. ./defs' foo.test 2>&1 1>&5 \
+env me=foo $SHELL -c '. ./defs' foo.test && exit 1
+env me=foo $SHELL -c '. ./defs' foo.test 2>&1 1>&5 \
   | grep "variable \`me' is set in the environment.*unsafe" || exit 1
 
 :
-- 
1.7.2.3