Hi Jim, thanks for the quick review. On 02/02/2012 07:58 PM, Jim Meyering wrote: > Stefano Lattarini wrote: >> On 02/02/2012 03:24 PM, Stefano Lattarini wrote: >>> On 02/02/2012 02:57 PM, Stefano Lattarini wrote: >>>> The attached patch should take care of three of the remaining five >>>> failures still present on latest Fedora (see automake bug#10418). >>>> I will push to master shortly if there is no objection. >>>> >>> Hmph, this causes several new and more serious failures. Let's drop >>> this patch. I hope I'll be able to come up with a correct solution >>> in the next days. >>> >> OK, turns out the failures in perl up to 5.12 were due to a limitation in >> the IPC::Open3. So let's keep the patch (slightly adjusted), but relax >> the tests a bit when a "defective" IPC::Open3 is detected. Attached is >> what I'll push shortly if there is no objection. > > I've just tested this on Fedora 16, and confirm that > it reduces to two the number of "make check" test failures. > Good! I'm thus closing this bug report, since the two remaining failures in 'depmod.tap' are already reported in bug#10434.
>> Subject: [PATCH] tap/perl: handle missing or non-executable scripts better >> >> This change improves how our Perl-based TAP driver handles >> non-runnable test scripts (meaning they might be not executable, >> or not readable, or even not exist). In particular, it makes the >> driver deterministically display a clear "ERROR" result instead >> of possibly dying with diagnostic from 'TAP::Parser' internals, >> and prevents it from displaying spurious "missing TAP plan" errors. >> >> Moreover, with this change, some testsuite failures present only >> with newer perl versions (e.g., 5.14) are fixed. See automake >> bug#10418. >> >> * tests/tap-bad-prog.tap: When testing the perl implementation of >> the TAP driver, and when the perl interpreter offers a good-enough >> 'IPC::Open3::open3' function, expect it not to display spurious >> "missing TAP plan" diagnostic if the error is actually due to a >> non-runnable test script. >> * lib/tap-driver.pl (start): Removed, broken up into ... >> (setup_io): ... this ... >> (setup_parser): ... and this, which now tries to catch and report >> errors in launching the test scripts. >> (finish): New, used by both 'main' and 'setup_parser'. >> (main): Adjust. > ... > >> # Check that no spurious test results is reported. This is lower-priority > > One nit in context: > > s/results/result/ > Fixed. >> -# (and in fact the check currently fails. >> -command_ok_ 'no spurious results' -D TODO -r 'still get "missing plan"' \ >> +# (and in fact the check currently fails for our awk-based driver). >> +directive= >> +if test $am_tap_implementation = shell; then >> + directive=TODO >> +else >> + # Older versions of IPC::Open3 (e.g., version 1.05 on perl 5.12.4 or >> + # version 1.0103 on perl 5.6.2) fails to properly trap errors in exec(2) > > One nit in a new comment: s/fails/fail/ > Ouch, amazing how may grammar mistakes I've managed to pile up in the last few days :-( Fixed this as well >> + # calls in the child process; hence, the TAP driver cannot be properly >> + # informed of such error. >> + if $PERL -w -e ' >> + use IPC::Open3 qw/open3/; >> + $@ = ""; >> + eval { open3(*STDIN, *STDOUT, *STDERR, "am--no-such-command") }; >> + $@ =~ m/\bopen3:.*am--no-such-command/ >> + or die "Bad \$@ value: \"$@\"\n"; >> + '; then >> + : # OK. IPC::Open3 should be good enough. >> + else >> + for s in '"missing plan" message' 'results'; do >> + skip_ -r "IPC::Open3 not good enough" "no spurious $s" >> + done > > Perhaps it's just your preferred style, but the quotes around 'results' > are unnecessary, so I would remove them. > I'd rather leave them, for consistency with the other item ('"missing plan" message'). Hope that's OK with you. Thanks, Stefano