[FFmpeg-devel] [PATCH] fate: give stderr hint when a test fails.

[Clément Bœsch]

On Sat, Dec 24, 2011 at 02:38:23AM +0100, Alexander Strasser wrote:
> Hi,
> 
>  I like your patch. Just a few comments.
> 
> Clément Bœsch wrote:
> > ---
> >  tests/fate-run.sh |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tests/fate-run.sh b/tests/fate-run.sh
> > index 350ff57..7f6b886 100755
> > --- a/tests/fate-run.sh
> > +++ b/tests/fate-run.sh
> > @@ -136,5 +136,9 @@ fi
> >  
> >  echo "${test}:${sig:-$err}:$($base64 <$cmpfile):$($base64 <$errfile)" >$repfile
> >  
> > -test $err = 0 && rm -f $outfile $errfile $cmpfile $cleanfiles
> > +if [ $err -eq 0 ]; then
> 
>   Any particular reason not to use "if test $err = 0; then"?
> 

None in particular, I just rewrote it that way when changing the if/else
form because it was more obvious to me. Restored to "if test $err = 0".

> > +    rm -f $outfile $errfile $cmpfile $cleanfiles
> > +else
> > +    echo "test failed, stderr output available here in $errfile"
> 
>   Is the "here" in the output really helpful? Maybe it could be just
> omitted. The whole message is a bit terse.
> 

Yes, as said in the other mail I dropped the "here" word.

>   Alternative suggestion:
>   "Warning: Test failed. Look at $errfile for details."
> 

Sounds even better, but I'll leave away the "Warning"; it's not a warning,
it's a miserable failure. Also added the $test name (useful with threaded
run)

...and pushed.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111226/3291feb3/attachment.asc>