[FYI] patch pushed (was: Re: [PATCH 2/2] Extended tests on AC_CONFIG_AUX_DIR).
I've now pushed the patch to master. Regards, Stefano
Re: [PATCH] Extend checks on remake rules.
On Thursday 16 December 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Thu, Dec 16, 2010 at 01:27:10AM CET: On Wednesday 15 December 2010, Ralf Wildenhues wrote: [ magic strings ] Why all the variation in these? That makes the tests harder to read. I'd rather not change the use of magic strings on the other remake tests, unless you insist. The test remak3a.test is IMHO simple enough not to cause confusion, and changing the tests remake8{a,b}.test would be quite cumbersome. I don't insist. Thanks. + test x'$(am_fingerprint)' = x'DummyValue' Why do you quote DummyValue here? Partly for symmetry with the left side, and partly (especially I'd say) to make the leading `x' stick out better as not a part of the expected value. The leading x should not be needed on either side. True, should not -- but I got in the habit of always using it, even where technically not needed, rather then risking to forgot it one time when it's really required. (several instances below) Should I remove them? (sorry if I ask, but the should above does not make it clear if you're asking to remove them or just noting that they are not really required). Ah, the fun of English negation and passive voice. I'm not totally firm in this area either, but I think that should not be needed has the meaning of is not needed, at least I think so rather than don't do this or I'll poke you with a stick. I think the same way too, but since you wrote ok with nits addressed in the beginning of your mail, I wasn't sure whether that should not were pointing out a nit (hence to be addressed). I hope somebody corrects me on this if my interpretation is wrong. Attached is also the amended patch. I will push in 72 hours if there are no more objections (and if all my testing on Linux and Solaris succeeds). I don't object, but TBH, I didn't look at the patch again. ;-) Thast shouldn't be really necessary if you've looked at my inline comments. Thanks! Ralf Thanks for the approval. I've now merged the patch into master, and pushed. Regards, Stefano
[FYI] patch pushed (was: [PATCH] Improve tests on generated portions of configure help screen.)
I've now pushed the patch to maint, and merged maint into master. Regards, Stefano
[FYI] patch pushed (was: Re: [PATCH] Improvements and extend tests on canonicalization.)
Pushed to master now. Regards, Stefano
[PATCH] {master} Improve and extend tests `suffix*.test'.
Hello automakers. Here it is one of the last (5 or 6) testsuite-only patches I have in my repository. Tested successfully: - Debian GNU/Linux, system GNU make (3.81) bash (4.1), gcc 4.4.4, libtool 2.2.6b, autoconf 2.68. - Debian GNU/Linux, GNU make 3.79, dash (0.5.5.1), gcc 4.0.1, libtool 2.2.6b, autoconf 2.68 - Debian GNU/Linux, GNU make 3.82, dash (0.5.2), gcc 2.95, libtool 2.2.6b, autoconf 2.68 - Debian GNU/Linux, GNU make 3.80, pdksh (5.2.14), gcc 3.3, libtool libtool 1.5.6, autoconf 2.65 - Debian GNU/Linux, GNU make 3.78.1, ksh (Version JM 93t+ 2009-05-01), SunStudio C compiler (Sun C 5.10 Linux_i386 2009/06/03), libtool 2.2.2, autoconf 2.62 - Debian GNU/Linux, FreeBSD make (from freebsd-buildutils 8.0), bash 2.05, gcc 3.4, libtool 2.2.2, autoconf 2.62 - Debian GNU/Linux, NetBSD make (from pmake-1.111 [sic]), bash 3.0, SunStudio C compiler (Sun C 5.10 Linux_i386 2009/06/03), libtool 2.2.8, autoconf 2.63 - Solaris 10, GNU make 3.82, Solaris /bin/sh, libtool 2.4, autoconf 2.68 SunStudio C compiler (Sun C 5.9 SunOS_i386 Patch 124868-15 2010/08/11), some GNU tools (sed, expr, awk) available Tested with some errors: - Solaris 10, CCS make, Solaris /bin/ksh, libtool 2.4, autoconf 2.68, SunStudio C compiler (Sun C 5.9 SunOS_i386 Patch 124868-15 2010/08/11) only Solaris utilities available - Solaris 10, XPG4 make, Solaris /bin/sh, libtool 2.4, autoconf 2.68, gcc 4.4.4, only Solaris utilities available - Solaris 10, dmake, Solaris /bin/sh, libtool 2.4, autoconf 2.68, gcc 4.4.4, some GNU tools (sed, expr, awk) available - Debian GNU/Linux, Heirloom make and shell (2007), gcc 4.2.4, libtool 2.2.8, autoconf 2.67, and with Heirloom utilities (sed, grep, etc.) preferred over system (GNU) utilities In all the setups indicated as with some errors above, the tests `suffix3.test', `suffix8.test', `suffix10.test' and `suffix11.test' failed due to the inability of those make implementations to chain implicit rules. I would have worked around those failures by either declaring hidden dependencies explicitly in the Makefile.am files, or by skipping the affected tests if the make implementation is found unable to chain implicit dependencies. But since there's no consensus on this yet, I'd rather leave the tests fail FTM. They can be adjusted in a follow-up patch anyway (or better, Automake could be improved to work around the limitations of those make implementations). OK to apply to a temporary branch off of maint, and merge to master? I will push in 72 hours (Sunday evening) if there is no objection or review by then. Regards, Stefano From 497eb149e4b470f00afe477ce8b5e2da63c61252 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Fri, 15 Oct 2010 17:37:38 +0200 Subject: [PATCH] Improve and extend tests `suffix*.test'. * tests/suffix.test: Check that suffix rules for C compilation are only included once. Try also with a static library. * tests/suffix2.test: Add a new grep to help potential debugging. Do not run automake with the `--add-missing' options, since we already create all the needed auxiliary files. Try also *without* the `no-dependencies' automake option. * tests/suffix4.test: Make grepping of Makefile.in stricter. * tests/suffix3.test: Rewritten to run also autoconf, ./configure and make. * tests/suffix5.test: Likewise. * tests/suffix6.test: Fix botched recipe indentation (eight spaces were used instead of a tabulation character). Extend to check that `.obj' is handled like `.$(OBJEXT)' (as is done for `.o'). Improved parsing grepping of generated Makefile.in. Other minor fixes and improvements. * tests/suffix10.test: Move some checks in Makefile.am. Also run make all. * tests/suffix12.test: Likewise, and account for VPATH issues in weaker make implementations. * tests/suffix11.test: Likewise. Also, run make distcheck, for completeness, and related changes. * tests/suffix8.test: Likewise. Also, do not put `gcc' anymore in $required. * tests/suffix13.test: Do not use the `--force-missing' automake option unnecessarily. * tests/suffix6b.test: New test, semantic sister of `suffix6.test'. * tests/suffix6c.test: Likewise. * tests/Makefile.am (TESTS): Updated. --- ChangeLog | 32 ++ tests/Makefile.am |2 + tests/Makefile.in |2 + tests/suffix.test | 14 ++-- tests/suffix10.test | 31 - tests/suffix11.test | 37 +--- tests/suffix12.test | 18 ++ tests/suffix13.test |2 +- tests/suffix2.test | 13 +-- tests/suffix3.test | 37 +++-- tests/suffix4.test |4 ++- tests/suffix5.test | 24 +++-- tests/suffix6.test | 37 +++- tests/suffix6b.test | 77 ++ tests/suffix6c.test | 92 +++ tests/suffix8.test | 67
Avoid false positive in sc_tests_plain_make maintainer-check.
I'm seeing this in master now: ../automake/tests/remake11.test:makefiles_am_list=`find . -name Makefile.am | LC_ALL=C sort` ../automake/tests/remake11.test:makefiles_list=`echo $makefiles_am_list | sed 's/\.am$//'` Do not run make in the above tests. Use $MAKE instead. Makefile:1090: recipe for target `sc_tests_plain_make' failed make: *** [sc_tests_plain_make] Error 1 Merging the patch below in the tests-remake-extend branch to avoid it. Thanks, Ralf Avoid false positive in sc_tests_plain_make maintainer-check. * Makefile.am (sc_tests_plain_make): Ensure to only match full `make' words. Avoid false positive with remake11.test. diff --git a/Makefile.am b/Makefile.am index ae096cb..15aa3b3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -342,7 +342,7 @@ sc_tests_make_without_am_makeflags: ## Tests should never call make directly. sc_tests_plain_make: - @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*make'; then \ + @if grep -v '^#' $(srcdir)/tests/*.test | $(EGREP) ':[ ]*make( |$$)'; then \ echo 'Do not run make in the above tests. Use $$MAKE instead.' 12; \ exit 1; \ fi diff --git a/Makefile.in b/Makefile.in index 04d7067..1c1bf81 100644 --- a/Makefile.in +++ b/Makefile.in @@ -1087,7 +1087,7 @@ sc_tests_make_without_am_makeflags: fi sc_tests_plain_make: - @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*make'; then \ + @if grep -v '^#' $(srcdir)/tests/*.test | $(EGREP) ':[ ]*make( |$$)'; then \ echo 'Do not run make in the above tests. Use $$MAKE instead.' 12; \ exit 1; \ fi
Re: [PATCH] Improvements and extend tests on canonicalization.
On Thursday 16 December 2010, Ralf Wildenhues wrote: --- /dev/null +++ b/tests/canon7.test @@ -0,0 +1,93 @@ +# Stress test on canonicalization. [...] +cat Makefile.am 'END' +noinst_PROGRAMS = dummy_static dummy_dynamic ,foo-bar +noinst_LIBRARIES = libb.az+baz.a +noinst_LTLIBRARIES = lib~zardoz,,.la + +dummy_static_SOURCES = dummy.c lib.h +dummy_dynamic_SOURCES = $(dummy_static_SOURCES) + +dummy_static_LDADD = $(noinst_LIBRARIES) +dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) + +_foo_bar_SOURCES = libs.c +libb_az_baz_a_SOURCES = libs.c +lib_zardoz___la_SOURCES = libd.c + +check-local: + ls -l + ./,foo-bar + ./dummy_static + ./dummy_dynamic + ./,foo-bar | grep 'Hello, FooBar!' + ./dummy_static | grep 'Hello from Static!' + ./dummy_dynamic | grep 'Hello from Dynamic!' +END + +cat foobar.c 'END' +#include stdio.h +int main(void) +{ + printf(Hello, FooBar!\n); + return 0; +} +END + +cat dummy.c 'END' +#include stdio.h +#include lib.h +int main(void) +{ + printf(Hello from %s!\n, dummy_func()); + return 0; +} +END + +echo 'char *dummy_func(void);' lib.h +echo 'char *dummy_func(void) { return Dynamic; }' libd.c +echo 'char *dummy_func(void) { return Static; }' libs.c I'm not sure how you tested this, but ',foo-bar' fails to link for me on GNU/Linux (and supposedly everywhere else, lacking a 'main' symbol). Yes, and I cant' understand how I could have missed that. I mean, I clearly remember running the tests after the squashed-in amendaments... (asking to self) What kind of blunder did I do? Anyway, sorry for the noise and thanks for fixing this. Just a nit below (inlined in your patch) ... Also, the code passes pointers to read-only memory (literal strings) in non-const pointers (literal strings may not be modifiable). Sorry again. My C-fu is not very strong :-( I'm pushing the fix below. Note that you're lucky using convenience archives here. With shared libraries, the const data would need special handling in order to work on w32. (See the Libtool documentation, git version, for details.) Thanks, Ralf Fix canon7.test failure. * tests/canon7.test (_foo_bar_SOURCES): Add foobar.c. (lib.h, libd.c, libs.c): Use const for constant strings. diff --git a/tests/canon7.test b/tests/canon7.test index 9b3d8d0..3f25d6f 100755 --- a/tests/canon7.test +++ b/tests/canon7.test @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c +_foo_bar_SOURCES = libs.c foobar.c _foo_bar_SOURCES = foobar.c should suffice here. libb_az_baz_a_SOURCES = libs.c lib_zardoz___la_SOURCES = libd.c @@ -76,9 +76,9 @@ int main(void) } END -echo 'char *dummy_func(void);' lib.h -echo 'char *dummy_func(void) { return Dynamic; }' libd.c -echo 'char *dummy_func(void) { return Static; }' libs.c +echo 'const char *dummy_func(void);' lib.h +echo 'const char *dummy_func(void) { return Dynamic; }' libd.c +echo 'const char *dummy_func(void) { return Static; }' libs.c libtoolize $ACLOCAL Thanks, Stefano
Re: [PATCH] Improvements and extend tests on canonicalization.
* Stefano Lattarini wrote on Sat, Dec 11, 2010 at 06:41:46PM CET: Ping on this? Reference: http://lists.gnu.org/archive/html/automake-patches/2010-09/msg00106.html The updated patch is attached. I will push it in 72 hours (by tuesday evening) unless there are objections. --- /dev/null +++ b/tests/canon7.test @@ -0,0 +1,93 @@ +# Stress test on canonicalization. [...] +cat Makefile.am 'END' +noinst_PROGRAMS = dummy_static dummy_dynamic ,foo-bar +noinst_LIBRARIES = libb.az+baz.a +noinst_LTLIBRARIES = lib~zardoz,,.la + +dummy_static_SOURCES = dummy.c lib.h +dummy_dynamic_SOURCES = $(dummy_static_SOURCES) + +dummy_static_LDADD = $(noinst_LIBRARIES) +dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) + +_foo_bar_SOURCES = libs.c +libb_az_baz_a_SOURCES = libs.c +lib_zardoz___la_SOURCES = libd.c + +check-local: + ls -l + ./,foo-bar + ./dummy_static + ./dummy_dynamic + ./,foo-bar | grep 'Hello, FooBar!' + ./dummy_static | grep 'Hello from Static!' + ./dummy_dynamic | grep 'Hello from Dynamic!' +END + +cat foobar.c 'END' +#include stdio.h +int main(void) +{ + printf(Hello, FooBar!\n); + return 0; +} +END + +cat dummy.c 'END' +#include stdio.h +#include lib.h +int main(void) +{ + printf(Hello from %s!\n, dummy_func()); + return 0; +} +END + +echo 'char *dummy_func(void);' lib.h +echo 'char *dummy_func(void) { return Dynamic; }' libd.c +echo 'char *dummy_func(void) { return Static; }' libs.c I'm not sure how you tested this, but ',foo-bar' fails to link for me on GNU/Linux (and supposedly everywhere else, lacking a 'main' symbol). Also, the code passes pointers to read-only memory (literal strings) in non-const pointers (literal strings may not be modifiable). I'm pushing the fix below. Note that you're lucky using convenience archives here. With shared libraries, the const data would need special handling in order to work on w32. (See the Libtool documentation, git version, for details.) Thanks, Ralf Fix canon7.test failure. * tests/canon7.test (_foo_bar_SOURCES): Add foobar.c. (lib.h, libd.c, libs.c): Use const for constant strings. diff --git a/tests/canon7.test b/tests/canon7.test index 9b3d8d0..3f25d6f 100755 --- a/tests/canon7.test +++ b/tests/canon7.test @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c +_foo_bar_SOURCES = libs.c foobar.c libb_az_baz_a_SOURCES = libs.c lib_zardoz___la_SOURCES = libd.c @@ -76,9 +76,9 @@ int main(void) } END -echo 'char *dummy_func(void);' lib.h -echo 'char *dummy_func(void) { return Dynamic; }' libd.c -echo 'char *dummy_func(void) { return Static; }' libs.c +echo 'const char *dummy_func(void);' lib.h +echo 'const char *dummy_func(void) { return Dynamic; }' libd.c +echo 'const char *dummy_func(void) { return Static; }' libs.c libtoolize $ACLOCAL
Re: [PATCH] Improvements and extend tests on canonicalization.
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 08:16:25PM CET: On Thursday 16 December 2010, Ralf Wildenhues wrote: --- a/tests/canon7.test +++ b/tests/canon7.test @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c +_foo_bar_SOURCES = libs.c foobar.c _foo_bar_SOURCES = foobar.c should suffice here. Feel free to fix that. Thanks! Ralf
Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.
* Stefano Lattarini wrote on Mon, Dec 13, 2010 at 07:54:05PM CET: OK to apply to a temporary branch off of maint, and merge to master? The patch is ok with nits addressed. BTW, notice that I'm planning to further extend the Lex/Yacc tests and make them more semantic, but that should be better done in a follow-up patch IMVHO. If there is any way I can get you to not do any more of such conversions: Please don't work even more on *these* tests. They are Good Enough[tm]. The incremental gain from more change is not worth the additional work from you nor review from me. Actually, lex and yacc support has *several* *real* issues, with maybe more than a dozen reported bugs in the last few years, many of them unfixed. See the list archives. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well (or is willing to learn) could look into some of the issues. I should warn that it's not easy though to come up with coherent/consistent and portable improvements. Subject: [PATCH] Extend, fix and improve tests on Lex and Yacc support. * tests/lexcpp.test: New test script, on support for Lex + C++. * tests/lexvpath.test: New test script, test build and rebuild rules for lexers in VPATH setup. * tests/yacc-basic.test: New test script, run simple semantic checks on basic Yacc support (similarly to what lex3.test does for Lex support). * tests/lex.test: Don't create useless dummy source file joe.l. Remove extra blank lines. (Makefile.am): Remove useless definition of $(LDADD). * tests/lex4.test: Add trailing `:' command. Do not create dummy useless lex source file. * tests/lex2.test: Likewise. Call automake with the `-a' option, so that it doesn't fail for the absence of `ylwrap' script. Make grepping of automake stderr stricter. * tests/yacc7.test: Add trailing `:' command. Enable `errexit' shell flag earlier (just after having sourced ./defs). * tests/yacc4.test: Likewise. Also ... (configure.in): Use pre-populated skeleton set up by ./defs, instead of writing one from scratch. Other minor cosmetic changes. * tests/yacc5.test: Likewise. * tests/yaccvpath.test: Likewise. Also ... ($distdir): New variable. Use it throughout. * tests/lex5.test: Likewise. * tests/lex3.test: Likewise. Check the distdir, rather than grepping the distribution tarball. Extend the test on the created binary, and be sure to avoid hangs. Add some comments. * tests/yacc.test: Use stricter grepping. Add trailing `:'. * tests/yacc6.test: Likewise. * tests/yacc3.test: Likewise. Prefer `cp -f' over plain `cp'. Do not create unused file `Makefile.sed'. Remove useless rules from Makefile.am. Other minor cosmetic changes. * tests/yacc2.test: Make grepping of generated `Makefile.in' and of automake error messages stricter. Do not redirect output of grep to /dev/null. Prefer `cp -f' over plain `cp'. Move call to aclocal earlier. Reduce the number of empty blank lines. Fix a typo in comments. * tests/yacc8.test: Fixed bugs that reduced the completeness of the tests. Added trailing `:' command. (configure.in): Use pre-populated skeleton set up by ./defs, instead of writing one from scratch. * tests/yaccpp.test: Test also extensions `.y++', `.ypp', and `.yxx', rather than only `.yy'. * tests/defs.in ($required): Better recognition of requirement flex. * tests/Makefile.am (TESTS): Update. --- a/tests/defs.in +++ b/tests/defs.in @@ -113,6 +113,13 @@ do echo $me: running etags --version -o /dev/null ( etags --version -o /dev/null ) || exit 77 ;; +flex) + # Since flex is required, we pick LEX for ./configure. + LEX=flex + export LEX + echo $me: running flex --version + ( flex --version ) || exit 77 + ;; I don't understand why the new 'required=flex' entry should be needed in defs.in. Any tests that fail without the defs.in change? +++ b/tests/lex.test @@ -26,22 +26,16 @@ END cat Makefile.am 'END' bin_PROGRAMS = zot zot_SOURCES = joe.l -LDADD = @LEXLIB@ Please don't remove that. It is documented to be required. END --- a/tests/lex3.test +++ b/tests/lex3.test +# Basic semantic checks on Lex support. # Test associated with PR 19. # From Matthew D. Langston. cat Makefile.am 'END' @@ -46,38 +44,44 @@ END cat foo.l 'END' %% -END return EOF; +GOOD return EOF; . %% int main () { - while (yylex () != EOF) -; - - return 0; + if (yylex () == EOF) +return 0; + else +return 1; That's not semantic either. One wouldn't write a lexer like that. } END +# Program should build and run. $MAKE -echo 'This is the END' | ./foo -$MAKE distcheck +echo GOOD | ./foo +echo BAD | ./foo Exit 1 --- a/tests/lex5.test +++ b/tests/lex5.test @@ -33,10 +29,9 @@ AC_OUTPUT END cat Makefile.am 'END' -AUTOMAKE_OPTIONS = subdir-objects -LDADD = @LEXLIB@ - -bin_PROGRAMS= foo/foo
Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.
On Thursday 16 December 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Dec 13, 2010 at 07:54:05PM CET: OK to apply to a temporary branch off of maint, and merge to master? The patch is ok with nits addressed. I've addressed almost all of them, but I have a doubt about one (see defs.in below) and I disagree with one (see lex3.test below). BTW, notice that I'm planning to further extend the Lex/Yacc tests and make them more semantic, but that should be better done in a follow-up patch IMVHO. If there is any way I can get you to not do any more of such conversions: Please don't work even more on *these* tests. They are Good Enough[tm]. IMHO they're not. See just below. The incremental gain from more change is not worth the additional work from you nor review from me. Actually, lex and yacc support has *several* *real* issues, with maybe more than a dozen reported bugs in the last few years, many of them unfixed. See the list archives. Yes, but I woldn't dare trying to modify the Lex/Yacc related code with the limited understanding I have of it, without having a *much* stronger testsuite than it's currently available. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well I'm not that somebody, unfortunately. (or is willing to learn) Well, maybe I might *try* to be this somebody... once I feel backed up by a strong-enough testsuite. could look into some of the issues. I should warn that it's not easy though to come up with coherent/consistent and portable improvements. Subject: [PATCH] Extend, fix and improve tests on Lex and Yacc support. * tests/lexcpp.test: New test script, on support for Lex + C++. * tests/lexvpath.test: New test script, test build and rebuild rules for lexers in VPATH setup. * tests/yacc-basic.test: New test script, run simple semantic checks on basic Yacc support (similarly to what lex3.test does for Lex support). * tests/lex.test: Don't create useless dummy source file joe.l. Remove extra blank lines. (Makefile.am): Remove useless definition of $(LDADD). * tests/lex4.test: Add trailing `:' command. Do not create dummy useless lex source file. * tests/lex2.test: Likewise. Call automake with the `-a' option, so that it doesn't fail for the absence of `ylwrap' script. Make grepping of automake stderr stricter. * tests/yacc7.test: Add trailing `:' command. Enable `errexit' shell flag earlier (just after having sourced ./defs). * tests/yacc4.test: Likewise. Also ... (configure.in): Use pre-populated skeleton set up by ./defs, instead of writing one from scratch. Other minor cosmetic changes. * tests/yacc5.test: Likewise. * tests/yaccvpath.test: Likewise. Also ... ($distdir): New variable. Use it throughout. * tests/lex5.test: Likewise. * tests/lex3.test: Likewise. Check the distdir, rather than grepping the distribution tarball. Extend the test on the created binary, and be sure to avoid hangs. Add some comments. * tests/yacc.test: Use stricter grepping. Add trailing `:'. * tests/yacc6.test: Likewise. * tests/yacc3.test: Likewise. Prefer `cp -f' over plain `cp'. Do not create unused file `Makefile.sed'. Remove useless rules from Makefile.am. Other minor cosmetic changes. * tests/yacc2.test: Make grepping of generated `Makefile.in' and of automake error messages stricter. Do not redirect output of grep to /dev/null. Prefer `cp -f' over plain `cp'. Move call to aclocal earlier. Reduce the number of empty blank lines. Fix a typo in comments. * tests/yacc8.test: Fixed bugs that reduced the completeness of the tests. Added trailing `:' command. (configure.in): Use pre-populated skeleton set up by ./defs, instead of writing one from scratch. * tests/yaccpp.test: Test also extensions `.y++', `.ypp', and `.yxx', rather than only `.yy'. * tests/defs.in ($required): Better recognition of requirement flex. * tests/Makefile.am (TESTS): Update. --- a/tests/defs.in +++ b/tests/defs.in @@ -113,6 +113,13 @@ do echo $me: running etags --version -o /dev/null ( etags --version -o /dev/null ) || exit 77 ;; +flex) + # Since flex is required, we pick LEX for ./configure. + LEX=flex + export LEX + echo $me: running flex --version + ( flex --version ) || exit 77 + ;; I don't understand why the new 'required=flex' entry should be needed in defs.in. Any tests that fail without the defs.in change? Not for me, but since we do something similar for bison, I was just trying to be consistent. Should I remove this hunk? +++ b/tests/lex.test @@ -26,22 +26,16 @@ END cat Makefile.am 'END' bin_PROGRAMS = zot zot_SOURCES = joe.l -LDADD = @LEXLIB@ Please don't remove that. It is documented to be required. OK (even if automake doesn't currently require it at automake-time). END ---
Re: [PATCH] Improvements and extend tests on canonicalization.
On Thursday 16 December 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Thu, Dec 16, 2010 at 08:16:25PM CET: On Thursday 16 December 2010, Ralf Wildenhues wrote: --- a/tests/canon7.test +++ b/tests/canon7.test @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c +_foo_bar_SOURCES = libs.c foobar.c _foo_bar_SOURCES = foobar.c should suffice here. Feel free to fix that. Below's what I pushed. I tested it with both GCC and Sun Studio C compiler, both before and after merging to master. I hope this will avoid me another embarassing blunder. Regards, Stefano -*-*-*- Minor cleanups in canon7.test. * tests/canon7.test (_foo_bar_SOURCES): Remove libs.c. (configure.in): Remove AC_PROG_CXX. --- ChangeLog |6 ++ tests/canon7.test |3 +-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9386d80..cb8c207 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2010-12-17 Stefano Lattarini stefano.lattar...@gmail.com + + Minor cleanups in canon7.test. + * tests/canon7.test (_foo_bar_SOURCES): Remove libs.c. + (configure.in): Remove AC_PROG_CXX. + 2010-12-16 Ralf Wildenhues ralf.wildenh...@gmx.de Fix canon7.test failure. diff --git a/tests/canon7.test b/tests/canon7.test index 3f25d6f..b85a835 100755 --- a/tests/canon7.test +++ b/tests/canon7.test @@ -23,7 +23,6 @@ set -e cat configure.in 'END' AC_PROG_CC -AC_PROG_CXX AC_PROG_RANLIB dnl: for static libraries AC_PROG_LIBTOOL dnl: for libtool libraries AC_OUTPUT @@ -43,7 +42,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c foobar.c +_foo_bar_SOURCES = foobar.c libb_az_baz_a_SOURCES = libs.c lib_zardoz___la_SOURCES = libd.c -- 1.7.1
Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 10:10:29PM CET: On Thursday 16 December 2010, Ralf Wildenhues wrote: BTW, notice that I'm planning to further extend the Lex/Yacc tests and make them more semantic, but that should be better done in a follow-up patch IMVHO. If there is any way I can get you to not do any more of such conversions: Please don't work even more on *these* tests. They are Good Enough[tm]. IMHO they're not. See just below. The incremental gain from more change is not worth the additional work from you nor review from me. Actually, lex and yacc support has *several* *real* issues, with maybe more than a dozen reported bugs in the last few years, many of them unfixed. See the list archives. Yes, but I woldn't dare trying to modify the Lex/Yacc related code with the limited understanding I have of it, without having a *much* stronger testsuite than it's currently available. The *current* tests are good enough for the current code. How many bugs have we found due to the testsuite frenzy in the last few months with its 200+ patches? I'd guess less than 5. That's an awful signal to noise ratio. How much have you gotten to know the Automake code base (outside of tests/) as a result of that? I don't know, I cannot guess, but normally one needs to work on code to get to know it really. Why not enhance the testsuite as you go along fixing bugs or otherwise improving code outside the testsuite? That ensures that we progress. Also, while working on the code, it is often clearer which semantics you are possibly changing; then, we can expose them in the testsuite as we stumble over them. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well I'm not that somebody, unfortunately. Don't give yourself up before starting. (or is willing to learn) Well, maybe I might *try* to be this somebody... once I feel backed up by a strong-enough testsuite. I don't believe this argumentation, for reasons stated above. Of course good testsuite coverage doesn't ever fully make up for knowing portability issues, semantics of tools, and searching history and archives regularly. That just comes with it. --- a/tests/defs.in +++ b/tests/defs.in @@ -113,6 +113,13 @@ do echo $me: running etags --version -o /dev/null ( etags --version -o /dev/null ) || exit 77 ;; +flex) + # Since flex is required, we pick LEX for ./configure. + LEX=flex + export LEX + echo $me: running flex --version + ( flex --version ) || exit 77 + ;; I don't understand why the new 'required=flex' entry should be needed in defs.in. Any tests that fail without the defs.in change? Not for me, but since we do something similar for bison, I was just trying to be consistent. Should I remove this hunk? Yes, please. We don't need unneeded code. cat foo.l 'END' %% -END return EOF; +GOOD return EOF; . %% int main () { - while (yylex () != EOF) -; - - return 0; + if (yylex () == EOF) +return 0; + else +return 1; That's not semantic either. One wouldn't write a lexer like that. Yes, but this will have IMVHO a lower risk of hanging due to possible errors/blunders in other parts of the testcase. Hanging which, BTW, would happen ... } END +# Program should build and run. $MAKE -echo 'This is the END' | ./foo -$MAKE distcheck +echo GOOD | ./foo +echo BAD | ./foo Exit 1 ... here. I think we should keep the lexer my stupid but safer way. Maybe; but *now*, that is a good time for adding a comment in main above, because that is one bit of unobvious code. (Unobvious is the reason why the code does not look nor work like a normal lexer.) OK with that fixed. # Don't redefine several times the same variable. -cp Makefile.am Makefile.src +cp -f Makefile.am Makefile.src Why should you need this change? Generally, I don't see why you ever need 'cp -f'. I dimly remember that I used to think there were cp implementations which might prompt if stdout/stderr is a tty and the dest file exists, unless the `-f' option is used.. That is true, but when you run a test, there is no tty involved. What am I missing? But I might be very mistaken here, and since you tell me the `-f' is useless ... No. It is useless *here*. Thanks, Ralf