Re: [PATCH 2/3] {testsuite-work} tests: can use also $SHELL to check shell scripts from `lib/'

[Stefano Lattarini]

On Tuesday 07 June 2011, Peter Rosin wrote:
> Den 2011-06-06 21:48 skrev Stefano Lattarini:
> > * tests/ar-lib.test: If the variable `$test_prefer_config_shell'
> > is set to "yes", run the script under test with configure-time
> > determined $SHELL, rather than with /bin/sh.
> > The `$test_prefer_config_shell' variable defaults to empty, but
> > can be overridden at runtime by the user, thus allowing more
> > coverage.
> > * tests/compile.test: Likewise.
> > * tests/compile2.test: Likewise.
> > * tests/compile3.test: Likewise.
> > * tests/compile4.test: Likewise.
> > * tests/compile5.test: Likewise.
> > * tests/compile6.test: Likewise.
> > * tests/instsh2.test: Likewise.
> > * tests/instsh3.test: Likewise.
> > * tests/mkinst3.test: Likewise.
> > * tests/missing.test: Likewise.
> > * tests/missing2.test: Likewise.
> > * tests/missing3.test: Likewise.
> > * tests/missing5.test: Likewise.
> > * tests/defs (get_shell_script): New subroutine, factoring out
> > code common to the tests above.
> > (xsi-lib-shell): If `$am_prefer_config_shell' is set to "yes",
> 
> $test_prefer_config_shell
>
Fixed.

> > index cf8d6cb..c7e8a0e 100755
> > --- a/tests/compile4.test
> > +++ b/tests/compile4.test
> > @@ -20,6 +20,8 @@
> >  required='cl'
> >  . ./defs || Exit 1
> >  
> > +get_shell_script compile
> > +
> 
> This test no longer checks if $AUTOMAKE -a copies over compile, as
> that is done manually now. I assume this aspect of $AUTOMAKE -a is
> tested elsewhere. Or is it?
>
Yes, in 'subobj.test'.

> > diff --git a/tests/defs b/tests/defs
> > index 37b5baa..1d50b1d 100644
> > --- a/tests/defs
> > +++ b/tests/defs
> > @@ -283,6 +283,23 @@ unindent ()
> >  }
> >  sed_unindent_prog="" # Avoid interferences from the environment.
> >  
> > +# get_shell_script SCRIPT-NAME
> > +# -----------------------------
> > +# Fetch an Automake-provided test script from the `lib/' directory into
> > +# the current directory, and, if the `$test_prefer_config_shell' variable
> > +# is set to "yes", modify its shebang line to use $SHELL instead of
> > +# /bin/sh.
> > +get_shell_script ()
> > +{
> > +  if test x"$test_prefer_config_shell" = x"yes"; then
> > +    sed "1s|#!.*|#! $SHELL|" "$top_testsrcdir/lib/$1" > "$1"
> > +    chmod a+x "$1"
> > +  else
> > +    cp "$top_testsrcdir/lib/$1" .
> > +  fi
> > +  sed 10q "$1" # For debugging.
> > +}
> > +
> 
> I'm not too fond of rewriting the scripts. Wouldn't it be better
> to execute them as they would be executed from Makefiles instead,
> and tweak $SHELL for the case where the shebang is desired to
> take effect?
> 
> I.e.
> if "$test_prefer_config_shell" = yes; then
>   TEST_SHELL=$SHELL
> else
>   TEST_SHELL=
> endif
> 
> and then
>  
> $TEST_SHELL ./compile ...
> 
> or something?
>
The point is that tweaking the shebang line in the scripts helps to
ensure that they are always run with $SHELL *unless explicitly told
otherwise*, while with your idiom they are run with /bin/sh *unless
$SHELL is used explicitly*.  The former scenario is safer for what
concerns coverage.

Also, by tweaking the test scripts, we reduce the amount of tweaking
necessary to have all script executions take place with $SHELL; see
how the diffs in this patch are much smaller and easier to follow
than those in my original patch?  I think this is a real advantage.

Does these motivations sway your judgement a bit?

> Doing it that way would also not remove the $AUTOMAKE -a tests.
>
Luckily, this is a non-issue, as such a check is already covered
by 'subobj.test'.

> Cheers,
> Peter
> 

Thank,
  Stefano