Re: [PATCH] {yacc-work} tests: more coverage on yacc/lex silent-rules, plus minor cleanups

[Ralf Wildenhues]

* Stefano Lattarini wrote on Fri, Jan 21, 2011 at 01:16:35PM CET:
> The `silent-yacc*.test' and `silent-lex*.test' tests were testing
> non-generic rules for C sources only, not for Lex/Yacc sources.
> 
> Also, the output emitted by automake-generated rules when updating
> a yacc-generated header wasn't being tested anywhere.

AFAICS it still isn't fully, even after this patch: you don't trigger
the "recover from removal of header" rule anywhere.  And if that wasn't
the point of the new test in this patch, then I don't see any reason
to add that new test; it can just be dropped.

> The attached patch should fix these shortcomings.
> 
> OK for the 'yacc-work' branch (branch which, BTW, might as well
> be made public at this point), to be merged into 'master'?

With above issue and nits below addressed.

> Subject: [PATCH] tests: more coverage on yacc/lex silent-rules, plus minor 
> cleanups
> 
> * tests/silent-yacc-gcc.test: Add sanity checks verifying that the
> generated Makefile.in files really contains the non-generic rules
> we expect.  Do not redundantly remove by hand files we know to be
> already removed "make maintainer-clean".
> (Makefile.am): Ensure we cover also non-generic yacc rules, by
> setting target-specific YFLAGS.
> (sub/Makefile.am): Likewise.
> * tests/silent-yacc-generic.test: Likewise.
> * tests/silent-lex-gcc.test: Likewise, but with LFLAGS instead of
> YFLAGS.
> * tests/silent-lex-generic.test: Likewise.
> * tests/silent-many-gcc.test: Likewise, but with both LFLAGS and
> YFLAGS.  Also ...
> (do_and_check_verbose_build): Remove redundant blank line.
> * tests/silent-many-generic.test: Likewise.
> * tests/silent-yacc-headers.test: New test.
> * tests/Makefile.am (TESTS): Update.

> --- a/tests/silent-lex-gcc.test
> +++ b/tests/silent-lex-gcc.test

> @@ -64,6 +66,18 @@ $ACLOCAL
>  $AUTOMAKE --add-missing
>  $AUTOCONF
>  
> +# Check that the expected non-generic rules has been truly generated.
> +# Otherwise, the coverage offered by this test will be weaker then
> +# expected and planned.

s/has been truly/actually have been/
s/then/than/

Ugh.  How about this shorter but yet more detailed comment instead?

# Ensure per-target rules are used, to ensure their coverage below.

Same in the other tests where this comment has been added.

> +$FGREP 'foo2-foo.c' Makefile.in
> +$FGREP 'foo2-foo2-foo.o' Makefile.in
> +$FGREP '$(foo2_LFLAGS)' Makefile.in
> +$FGREP '$(foo2_CFLAGS)' Makefile.in
> +$FGREP 'bar2-bar.c' sub/Makefile.in
> +$FGREP 'bar2-bar2-bar.o' sub/Makefile.in
> +$FGREP '$(bar2_LFLAGS)' sub/Makefile.in
> +$FGREP '$(bar2_CFLAGS)' sub/Makefile.in

Do you *really* think you need to do all of these greps to ensure that
things are actually working the same way they have for the last decade?
Won't just one, or maybe two, suffice?  I mean, come on, the only thing
I can see coming from this is that, if there ever arises the need to
change the object file naming, the person doing that change will be
cursing no end when they have to adjust such a lot of code.  No bug will
ever be fixed or avoided from this.

Same for all the other tests.

>  # Force gcc ("fast") depmode.
>  # This apparently useless "for" loop is here to simplify the syncing
>  # with sister test `silent-lex-gcc.test'.

[...]

> --- /dev/null
> +++ b/tests/silent-yacc-headers.test

> +# Check silent-rules mode for Yacc, when used with the `-d' option.
> +# Keep this in sync with sister test `silent-yacc-d-gcc.test'.
> +
> +required=yacc
> +. ./defs || Exit 1
> +
> +set -e
> +
> +mkdir sub
> +
> +cat >>configure.in <<'EOF'
> +AM_SILENT_RULES
> +AC_PROG_YACC
> +AC_PROG_CC
> +AC_OUTPUT
> +EOF
> +
> +cat > Makefile.am <<'EOF'
> +# Need generic and non-generic rules.
> +AM_YFLAGS = -d
> +bin_PROGRAMS = foo bar
> +foo_SOURCES = parse.y
> +bar_SOURCES = $(foo_SOURCES)
> +bar_YFLAGS = $(AM_YFLAGS)
> +EOF
> +
> +cat > parse.y <<'EOF'
> +%{
> +void yyerror (char *s) {}
> +int yylex (void) {return 0;}
> +int main(void) {return 0;}

int main (void) { return 0; }
similar above

> +%}
> +%token EOF
> +%%
> +fubar : 'f' 'o' 'o' 'b' 'a' 'r' EOF {};
> +EOF