Re: [PATCH 5/5] Improved test silent5.test.

[Ralf Wildenhues]

Hi Stefano,

* Stefano Lattarini wrote on Thu, Apr 22, 2010 at 10:31:24PM CEST:
> Subject: [PATCH 5/5] Improved test silent5.test.
> 
> * tests/silent5.test: Removed by hand all generated C files after
> non-verbose builds, to ensure the following builds are clean.
> A couple of greps of make's output have been made stricter.
> Extended test description telling it's fine if it overlaps a bit
> with other `silent*.test' tests.

> --- a/tests/silent5.test
> +++ b/tests/silent5.test

> @@ -116,7 +119,7 @@ do
>    $MAKE >stdout || { cat stdout; Exit 1; }
>    cat stdout
>    grep ' -c' stdout && Exit 1
> -  grep ' -o ' stdout && Exit 1
> +  grep ' -o' stdout && Exit 1

This test has become less instead of more strict; oversight or purpose?

>    grep mv stdout && Exit 1
>  
>    grep 'CXX .*foo1\.' stdout
> @@ -139,10 +142,13 @@ do
>    grep 'CXXLD .*baz' stdout
>    grep 'CCLD .*bla' stdout
>  
> +  # Ensure a clean rebuild.
>    $MAKE clean
> +  rm -f foo5.c foo6.[ch] sub/baz5.c sub/baz6.[ch]

Hmm, I wonder if doing clean and rebuild with the same V flag (and
without removing generated sources in between) would be a sensible
addition on top of this: it might trigger a different set of rules
(as I think you may be able to see with heirloom make).

>    $MAKE V=1 >stdout || { cat stdout; Exit 1; }
>    cat stdout
> -  grep ' -c' stdout
> +  grep ' -c ' stdout
>    grep ' -o ' stdout
>  
>    grep 'CXX .*foo1\.' stdout && Exit 1
> @@ -164,8 +170,12 @@ do
>    grep 'CCLD .*bar' stdout && Exit 1
>    grep 'CXXLD .*baz' stdout && Exit 1
>    grep 'CCLD .*bla' stdout && Exit 1
> +
> +  # Ensure a clean reconfiguration/rebuild.
>    $MAKE clean
>    $MAKE maintainer-clean
> +  rm -f foo5.c foo6.[ch] sub/baz5.c sub/baz6.[ch]

Wait, maintainer-clean should have removed all these files at this
point (and some of the other lex/yacc tests should have this tested,
too).  Does that not work for you?

Other than the above nits, this series looks fairly good to me.  You
could add something like 'Split out from silent5.test' to the test
descriptions, but that's a really minor point.

Did you run 'make maintainer-check' on the series?

Thanks for working on this,
Ralf