Re: testsuite failures when test scripts are run with zsh

[Stefano Lattarini]

Hi Ralf.

A new version of the patch is attached. It's still not definitive,
but I hope most things are settled by now.

I tested it with `zsh3 -o no_function_argzero' (passes all tests)  and
with heirloom-sh (passes all tests but vala4.test, but this is not a
regression, as you should remember:
http://lists.gnu.org/archive/html/automake-patches/2009-10/msg00024.html

If you say this version of the patch is OK to go, I'll test it a bit more
thoroughly.

> > [CUT]
> > I see your point.  It's OK with me to have `run_command' calling
> > Exit on failures, since (as you stressed) that's by far the most
> > common scenario.  However, we should then really add a similar
> > subroutine (say `run_redirect' -- tell me if you have a better
> > name) which only takes care of portably redirecting stdout/stderr
> > of a command (and, obviously, rewrite `run_command'
> > implementation to advantage of the new function).
> > Is this OK with you?
>
> This is making things too complicated for my taste.
After some thinking, I agree.  See below.

> Your run_command already has an -e option.  If we're going to
> return the exit status anyway from the function, then we wouldn't
> need that -e *at all*, we could just return whatever status the
> command caused.
>
> So if it pleases you better, then we can change it to either of the
> following:
>
> - '-e ignore' ignores the status of the command; run_command
> returns it;
> - no '-e' just lets run_command return the status of the command,
> rather than checking the command against zero.
> I prefer the first, but only on the grounds that more typing is
> done only in the rare case.
I thought about it, and now I strongly agree with you: adding another
subroutine is going to make things unnecessarly complicated.  But I'd
prefer that `run_command -e IGNORE' continues to do what it states,
e.g. *ignore* the failures instead of making the test case abort if
`set -e' is on.  Instead, what about adding support for a new special
argument to `-e', say `RETURN' (which is what I did in the attached
patch)?  It's easy, far less obtrusive than adding another function,
and it also keeps the "extra-typing" low (no need to ever add `-e 0').
Objections?

> [CUT]
>
> > Well, even if we are going to make `set -e' the default, I think
> > that this change should be introduced with patch.
>
> Do you mean "should be introduced with a separate patch"? 
Yes.  I somehow managed to mess up the wording in the mail.  Sorry.
> If yes, then I agree.
Good.

>
> > > BTW, now that we have TEST_LOG_COMPILER, and correctly unset it
> > > in defs.in, too, we can set it in tests/Makefile.am and worry
> > > less about shells like Solaris /bin/sh.  Of course, that would
> > > require us to warn that running tests directly (i.e., without
> > > 'make' in-between), might require to use a decent shell.
> >
> > Or we could add proper code in `tests/defs.in', to make it
> > re-execute the test scripts with CONFIG_SHELL.  But this should
> > be obviously done in a distinct patch.
> Yes.
OK.  This could be brought up in a new thread on the list.  I think it's
not a compelling change ATM, though.

> > --- a/tests/defs.in
> > +++ b/tests/defs.in
> > @@ -395,26 +395,91 @@ is_newest ()
> >    test -z "$is_newest_files"
> >  }
> >
> > +# run_command [-e STATUS] [-i FILE] [-m] [--] COMMAND
> > [ARGUMENTS..] +#
> > -----------------------------------------------------------------
> > +# Run the given COMMAND with ARGUMENTS and fail if COMMAND does
> > not exit +# with STATUS.  If status is "VOID" or "IGNORE", any
> > exit value of the +# command is acceptable.  If STATUS is "FAIL",
> > then any exit value of the +# command *but 0* is acceptable. 
> > Default STATUS is `0'. +# Also, save standard output and standard
> > error from COMMAND, by default +# respectively in files `stdout'
> > and `stderr' (in the current directory), +# or togheter in the
> > file `stdall' (in the current directory) if the `-m'
>
> typo togheter
Fixed.

> > +# option is given.
> > +run_command ()
> > +{
> > +  set +x # xtrace verbosity temporarly disabled in `run_command'
> > +  run_exitcode_expected=0
> > +  run_mix_stdout_and_stderr=no
> > +  while test $# -gt 0; do
> > +    case $1 in
> > +      -e) run_exitcode_expected=$2; shift;;
> > +      -m) run_mix_stdout_and_stderr=yes;;
> > +      --) shift; break;;
> > +      -?) echo "run_commmand(): invalid switch \`$1'" >&2; Exit
> > 99;;
>
> s/()//
Done.

>
> > +       *) break;;
> > +    esac
> > +    shift
> > +  done
> > +  case $# in
> > +    0) echo "run_command(): missing COMMAND argument" >&2; Exit
> > 99;;
>
> Likewise.
Done.

Regards,
    Stefano

0001-Testsuite-avoid-Zsh-related-problems-with-set-x.patch
Description: Text Data