From 265671e4e2cbd32efaf71595db19b37f97290b69 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini Date: Sat, 6 Aug 2011 20:21:18 +0200 Subject: [PATCH] tap: scripts with a SKIP plan but with exit status != 0 must error This change has been motivated by Automake's own testsuite. Some TAP tests there were erroring out (due to unexpected, unhandled failures) before having encountered TAP result, so that the simple-minded shell library implementing TAP generation ended up printing a "1..0" trailing test plan; this caused the script to be reported as a SKIP rather than an ERROR -- a nasty false negative. * lib/tap-driver: Add prototypes for each subroutine, to free up the order in which they can be defined and called. (main): Move the code checking for a bad exit status of the TAP producer ... (finish): ... here, and flush the TAP stream to ensure that the parser always obtains the producer's exit status. * tests/tap-skip-whole-badexit.test: New test. * tests/Makefile.am (tap_with_common_setup_tests): Add it. --- ChangeLog | 18 +++++++++ lib/tap-driver | 61 +++++++++++++++++++++++-------- tests/Makefile.am | 1 + tests/Makefile.in | 1 + tests/tap-skip-whole-badexit.test | 52 ++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 15 deletions(-) create mode 100755 tests/tap-skip-whole-badexit.test diff --git a/ChangeLog b/ChangeLog index e07ef47ab..75ea7a23c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2011-08-06 Stefano Lattarini + + tap: scripts with a SKIP plan but with exit status != 0 must error + This change has been motivated by Automake's own testsuite. Some + TAP tests there were erroring out (due to unexpected, unhandled + failures) before having encountered TAP result, so that the + simple-minded shell library implementing TAP generation ended up + printing a "1..0" trailing test plan; this caused the script to be + reported as a SKIP rather than an ERROR -- a nasty false negative. + * lib/tap-driver: Add prototypes for each subroutine, to free up + the order in which they can be defined and called. + (main): Move the code checking for a bad exit status of the TAP + producer ... + (finish): ... here, and flush the TAP stream to ensure that the + parser always obtains the producer's exit status. + * tests/tap-skip-whole-badexit.test: New test. + * tests/Makefile.am (tap_with_common_setup_tests): Add it. + 2011-08-06 Stefano Lattarini tap: fix whitespace munging of diagnostic messages diff --git a/lib/tap-driver b/lib/tap-driver index d59ae01ea..d836b5f6c 100755 --- a/lib/tap-driver +++ b/lib/tap-driver @@ -83,6 +83,34 @@ Getopt::Long::GetOptions ( 'ignore-exit' => sub { $cfg{"ignore-exit"} = 1; }, ) or exit 1; +# ------------- # +# Prototypes. # +# ------------- # + +sub add_test_result ($); +sub bool_opt ($$); +sub colored ($$); +sub copy_in_global_log (); +sub decorate_result ($); +sub extract_tap_comment ($); +sub finish (); +sub get_global_test_result (); +sub get_tap_line (); +sub get_test_results (); +sub handle_tap_bailout ($); +sub handle_tap_plan ($); +sub handle_tap_test ($); +sub main (@); +sub must_recheck (); +sub peek_tap_line (); +sub report ($;$); +sub start (@); +sub stringify_test_result ($); +sub testuite_error ($); +sub unget_tap_line ($); +sub write_test_results (); +sub yn ($); + # -------------- # # Subroutines. # # -------------- # @@ -204,6 +232,24 @@ sub start (@) sub finish () { + # Flush all the remaining TAP stream, so that we can obtain the + # exit status of the TAP producer. + do {} while defined get_tap_line (); + # TODO: we should probably use $parser->wait here, to catch signals too + if ($parser->exit != 0) + { + my $msg = sprintf "exited with status %d", $parser->exit; + if ($cfg{"ignore-exit"}) + { + # Log the exit status of the script anyway, even if it is not + # considered to be an error, to help debugging. + print "INFO: $test_script_name - $msg\n"; + } + else + { + testuite_error $msg; + } + } write_test_results; close LOG or die "closing $log_file: $!\n"; exit 0; @@ -400,21 +446,6 @@ sub main (@) testuite_error (sprintf "too %s tests run (expected %d, got %d)", $bad_amount, $planned, $run); } - # TODO: we should probably use $parser->wait here, to catch signals too - if ($parser->exit != 0) - { - my $msg = sprintf "exited with status %d", $parser->exit; - if ($cfg{"ignore-exit"}) - { - # Log the exit status of the script anyway, even if it is not - # considered to be an error, to help debugging. - print "INFO: $test_script_name - $msg\n"; - } - else - { - testuite_error $msg;; - } - } finish; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 7b3b77797..4e7cbda41 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1177,6 +1177,7 @@ tap-recheck-logs.test \ tap-skip-whole-whitespace.test \ tap-skip-whole.test \ tap-skip-whole-lastline.test \ +tap-skip-whole-badexit.test \ tap-todo-skip-together.test \ tap-todo-skip-whitespace.test \ tap-todo-skip.test \ diff --git a/tests/Makefile.in b/tests/Makefile.in index f03ec163a..12ba08ff1 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -1413,6 +1413,7 @@ tap-recheck-logs.test \ tap-skip-whole-whitespace.test \ tap-skip-whole.test \ tap-skip-whole-lastline.test \ +tap-skip-whole-badexit.test \ tap-todo-skip-together.test \ tap-todo-skip-whitespace.test \ tap-todo-skip.test \ diff --git a/tests/tap-skip-whole-badexit.test b/tests/tap-skip-whole-badexit.test new file mode 100755 index 000000000..76ba6925f --- /dev/null +++ b/tests/tap-skip-whole-badexit.test @@ -0,0 +1,52 @@ +#! /bin/sh +# Copyright (C) 2011 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# TAP support: +# - an exit status != 0 of a test script causes an hard error, even if +# the last line of output is a "SKIP plan" (e.g., "1..0 # SKIP"). + +parallel_tests=yes +. ./defs || Exit 1 + +echo TESTS = one.test two.test > Makefile.am + +. "$testsrcdir"/tap-setup.sh || fatal_ "sourcing tap-setup.sh" + +cat > one.test <<'END' +#!/bin/sh +echo '1..0 # SKIP' +exit 1 +END + +cat > two.test <<'END' +#!/bin/sh +echo '1..0' +exit 22 +END + +chmod a+x one.test two.test + +$MAKE check >stdout && { cat stdout; Exit 1; } +cat stdout + +# The 'prove' utility reports both the skip and the non-zero exit status, +# so we do the same. +count_test_results total=4 pass=0 fail=0 xpass=0 xfail=0 skip=2 error=2 + +grep '^ERROR: one\.test - exited with status 1$' stdout +grep '^ERROR: two\.test - exited with status 22$' stdout + +: -- 2.47.2