[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: testsuite failures when test scripts are run with zsh
From: |
Ralf Wildenhues |
Subject: |
Re: testsuite failures when test scripts are run with zsh |
Date: |
Fri, 16 Oct 2009 07:23:22 +0200 |
User-agent: |
Mutt/1.5.20 (2009-08-09) |
Hi Stefano,
* Stefano Lattarini wrote on Tue, Oct 13, 2009 at 04:24:38PM CEST:
> At Tuesday 13 October 2009, Ralf Wildenhues wrote:
> >
> > First off, I think that run_command really should Exit when the
> > command does not produce the intended status. It should not be
> > necessary to do run_command -e 1 $command || Exit 1
> >
> > That is much safer, and less maintenance.
> 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. 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.
> > If we really need to run some command where we need to ignore
> > the exit status, then we can still use
> >
> > run_command '$command || :'
> This is wrong, as currently `run_command' do not `eval' its COMMAND.
> And the exit status of $command would be lost, which might not be what
> we really want.
OK, sorry about that glitch.
> > Which brings me to the second inconsistent issue with this API: the
> > 'eval'uation level of the command is part of the API.
> > This is important, because when the absolute source and build
> > directories contain white space in the name (and Automake mostly
> > works in this case now), we should be doing the right thing.
> I don't understand. The `eval' used in the run_command implementation
> is there just to provide a shortand: no argument passed by the caller
> is ever eval'd (and this is the right thing to do, I think).
OK.
> > Then to your question above: yes it is ok to replace all instances
> > of AUTOMAKE_run and AUTOMAKE_fails (there is no need to replace
> > plain $AUTOMAKE without redirection).
> >
> > > If you think about it, the testsuite don't have `ACLOCAL_run' or
> > > `ACLOCAL_fails', but simply uses `run_command $ACLOCAL' and
> > > `run_COMMAND -e 1 $ACLOCAL'.
> > > By the way, this change should require a small change in
> > > `tests/README' too.
> > > If you agree with it, I think it should be done with a distinct
> > > patch.
> > Sounds good.
> Mmhh, I see another possible misunderstanding creeping in here.
> Better to clear it out, just to be absolutely sure. What I was trying
> to say is that we should get rid of AUTOMAKE_run and AUTOMAKE_fails,
> not add ACLOCAL_run and ACLOCAL_fails (especially now that I'm going
> to follow your advice and make run_command use `Exit 1' on failures).
> Is this OK?
OK.
> > > > BTW, your run_command doesn't do what it advertizes to do: it
> > > > doesn't necessarily cause a test failure when it should, esp.
> > > > when a test doesn't use 'set -e'.
> > >
> > > I think that here there's a misunderstanding about the meaning of
> > > `fail': you mean "the testcase fails", while I mean "the function
> > > fails", i.e. it return a value != 0. Do you have a rewording to
> > > suggest to make things clearer?
> >
> > "fail" is FAIL, and exit status != 0 is returning a nonzero exit
> > status.
> This will become mostly a moot issue once run_command will call
> `Exit 1' on unexpected exit status. Should I anyway use "FAIL"
> instead of "fail"?
Either way is fine.
> > > > > By the way, your observation has made me think: wouldn't it
> > > > > be better to enable `set -e' in defs.in, so that all the test
> > > > > cases could have a more uniform environment?
> > > >
> > > > This would require an audit of all tests that currently don't
> > > > use set -e. This needs testing on Solaris, OpenBSD, NetBSD,
> > > > Tru64, because of bugs and warts in their shell's
> > > > implementation of errexit.
> > >
> > > Unfortunately, I don't have access to any of those system, so
> > > I'll have to drop the ball on this.
> >
> > I can test the final iteration of this patch.
> 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"? If yes, then
I agree.
> > 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.
> --- 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
> +# 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/()//
> + *) break;;
> + esac
> + shift
> + done
> + case $# in
> + 0) echo "run_command(): missing COMMAND argument" >&2; Exit 99;;
Likewise.
> + *) run_cmd=$1; shift;;
> + esac
> + if test x"$run_mix_stdout_and_stderr" = x"yes"; then
> + run_evald_cmd='"$run_cmd" ${1+"$@"} >stdall 2>&1'
> + else
> + run_evald_cmd='"$run_cmd" ${1+"$@"} >stdout 2>stderr'
> + fi
> + if eval "$run_evald_cmd"; then
> + run_exitcode_got=0
> + else
> + run_exitcode_got=$?
> + fi
> + if test x"$run_mix_stdout_and_stderr" = x"yes"; then
> + set -x
> + cat stdall
> + { set +x; } 2>/dev/null
> + else
> + set -x
> + cat stderr >&2
> + cat stdout
> + { set +x; } 2>/dev/null
> + fi
> + case $run_exitcode_expected in
> + VOID|void|IGNORE|ignore|IGNORED|ignored|$run_exitcode_got)
> + run_rc=0
> + ;;
> + FAIL|fail|FAILURE|failure)
> + if test $run_exitcode_got -gt 0; then
> + run_rc=0
> + else
> + run_rc=1
> + fi
> + ;;
> + *)
> + run_rc=1
> + ;;
> + esac
> + echo "run_command: exit status $run_exitcode_got (expecting" \
> + "$run_exitcode_expected)"
> + set -x # reactivating temporarly turned-off xtrace verbosity
> + return $run_rc
> +}
Thanks,
Ralf
- Re: testsuite failures when test scripts are run with zsh, Ralf Wildenhues, 2009/10/08
- Re: testsuite failures when test scripts are run with zsh, Stefano Lattarini, 2009/10/09
- Re: testsuite failures when test scripts are run with zsh, Stefano Lattarini, 2009/10/09
- Re: testsuite failures when test scripts are run with zsh, Ralf Wildenhues, 2009/10/10
- Re: testsuite failures when test scripts are run with zsh, Ralf Wildenhues, 2009/10/13
- Re: testsuite failures when test scripts are run with zsh, Stefano Lattarini, 2009/10/13
- Re: testsuite failures when test scripts are run with zsh,
Ralf Wildenhues <=
- Re: testsuite failures when test scripts are run with zsh, Stefano Lattarini, 2009/10/16
- Re: testsuite failures when test scripts are run with zsh, Stefano Lattarini, 2009/10/23
- Re: testsuite failures when test scripts are run with zsh, Stefano Lattarini, 2009/10/28