Re: [PATCH] tests: avoid possibly undeserved PASS from check8.test

[Stefano Lattarini]

On 01/27/2012 10:32 AM, Peter Rosin wrote:
> Hi!
>
Hi Peter, thanks for the patch.

> Due to some confusion, I happened to look at check8.log from a successful
> run when I thought it had failed.  Anyway, I noticed a weakness in this
> output, quoting from the end of check8.log:
> 
> ...
> make  check-TESTS
> make[1]: Entering directory `/home/peda/automake/tests/check8.dir/build'
> XPASS: foo
> PASS: sub/foo
> PASS: bar.exe
> PASS: sub/bar.exe
> FAIL: baz.exe
> XFAIL: sub/baz.exe
> ===========================================================
> 2 of 6 tests did not behave as expected (1 unexpected pass)
> ===========================================================
> make[1]: Leaving directory `/home/peda/automake/tests/check8.dir/build'
> + grep 'XPASS.*foo$' stdout
> XPASS: foo
> + grep '^[^X]*PASS.*sub/foo$' stdout
> PASS: sub/foo
> + grep '^[^X]*PASS.*bar' stdout
> PASS: bar.exe
> PASS: sub/bar.exe
> + grep '^[^X]*PASS.*sub/bar' stdout
> PASS: sub/bar.exe
> + grep '^[^X]*FAIL.*baz' stdout
> FAIL: baz.exe
> + grep 'XFAIL.*sub/baz' stdout
> XFAIL: sub/baz.exe
> + :
> + exit_status=0
> + set +e
> + cd /home/peda/automake/tests
> + case $exit_status,$keep_testdirs in
> + find check8.dir -type d '!' -perm -700 -exec chmod u+rwx '{}' ';'
> + rm -rf check8.dir
> + test 0 '!=' 0
> + echo 'check8: exit 0'
> check8: exit 0
> + exit 0
> 
> 
> See how "grep '^[^X]*PASS.*bar' stdout" finds two matching lines?
> 
> 
> Here's a fix, ok for maint?
>
Below I have a suggestion to improve the fix.

> Or is it just too bleeding unlikely that things will fail here
> when they passed muster in the colorized check done a few lines
> up in check8.test?
>
Probably it is unlikely, but I still like if we can improve "theoretical
correctness" with a simple patch anyway.

> From edaca55fc2bb826fc28e37f382db06496f3abf5e Mon Sep 17 00:00:00 2001
> From: Peter Rosin <address@hidden>
> Date: Fri, 27 Jan 2012 10:28:25 +0100
> Subject: [PATCH] tests: avoid possibly undeserved PASS from check8.test
> 
> * tests/check8.test: Adjust grep pattern to not match "sub/bar"
> when looking for "bar" during the uncolorized tests.
> ---
>  tests/check8.test |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/check8.test b/tests/check8.test
> index dc8d3dd..327b837 100755
> --- a/tests/check8.test
> +++ b/tests/check8.test
> @@ -91,7 +91,7 @@ cat stdout
>  # Note: we are not grepping for the space here, due to the Solaris make VPATH
>  # rewriting (if we fix that, we can still write a separate test for it).
>
Since the VPATH rewrites only applies to tests from the srcdir, we need to
worry about it only for the 'foo' tests, not for the other tests (that are
anyway compiled in the builddir).  So I say ...

>  grep 'XPASS.*foo$' stdout
>  grep '^[^X]*PASS.*sub/foo$' stdout
>
... we leave these untouched ...

> grep '^[^X]*PASS.*bar' stdout
> grep '^[^X]*PASS.*sub/bar' stdout
> grep '^[^X]*FAIL.*baz' stdout
> grep 'XFAIL.*sub/baz' stdout
>
... but change these ...

> grep '^[^X]*PASS.* bar' stdout
> grep '^[^X]*PASS.* sub/bar' stdout
> grep '^[^X]*FAIL.* baz' stdout
> grep 'XFAIL.* sub/baz' stdout
>
... to these.

And maybe we update the comment above to match, like:

  # Note: we are not grepping for the space in the lines from the 'foo' tests,
  # due to the Solaris make VPATH rewriting (if we fix that, we can still write
  # a separate test for it).

WDYT?  Do you feel like tweaking your patch long these lines?

Thanks,
  Stefano