bug#10374: 3 test failures on fedora 16

[Jim Meyering]

Stefano Lattarini wrote:
> On 12/26/2011 11:26 PM, Jim Meyering wrote:
>> FAIL: tap-no-spurious-w
>>
> This is due to a backward-incompatible change in the newer TAP::Harness
> releases.  The attached patch (thoroughly commented) fixes this, and also
> makes the behaviour of our awk TAP driver consistent with that of these
> newer TAP::Harness releases.
>
> OK for master?

Suggestions for log and comments.

> Subject: [PATCH] tap/awk: "Bail out!" recognized also after leading
>  whitespace
>
> Newer versions of TAP::Harness (e.g., 3.23 on Perl 5.14.1)
> recognize a "Bail out!" directive also when it is prepended by
> leading whitespace; this was not the case for older TAP:Harness
> versions, (e.g., for version 3.17 on Perl 5.12.4), and for our
> TAP driver implemented in awk.
>
> * lib/tap-driver.sh: Handle "Bail out!" directive also when they
> are preceded by leading whitespace.

Grammar fix:

    * lib/tap-driver.sh: Handle the "Bail out!" directive also when it
    is preceded by leading whitespace.

> * tests/tap-spurious.test: Remove the tests checking that a
> "Bail out!" string coming right after leading whitespace does
> not trigger a bailout action.
> * tests/tap-bailout-leading-space.test: New test.
> * tests/list-of-tests.mk: Add it.
> Problem reported by Jim Meyering in automake bug#10374.
> ---
>  ChangeLog                            |   17 ++++++++
>  lib/tap-driver.sh                    |    8 +++-
>  tests/list-of-tests.mk               |    1 +
>  tests/tap-bailout-leading-space.test |   72 
> ++++++++++++++++++++++++++++++++++
>  tests/tap-no-spurious.test           |    5 ++-
>  5 files changed, 100 insertions(+), 3 deletions(-)
>  create mode 100755 tests/tap-bailout-leading-space.test
>
> diff --git a/ChangeLog b/ChangeLog
> index 53197a9..d551a28 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,22 @@
>  2011-12-27  Stefano Lattarini  <address@hidden>
>
> +     tap/awk: "Bail out!" recognized also after leading whitespace
> +     Newer versions of TAP::Harness (e.g., 3.23 on Perl 5.14.1)
> +     recognize a "Bail out!" directive also when it is prepended by
> +     leading whitespace; this was not the case for older TAP:Harness
> +     versions, (e.g., for version 3.17 on Perl 5.12.4), and for our
> +     TAP driver implemented in awk.
> +     * lib/tap-driver.sh: Handle "Bail out!" directive also when they
> +     are preceded by leading whitespace.
> +     * tests/tap-spurious.test: Remove the tests checking that a
> +     "Bail out!" string coming right after leading whitespace does
> +     not trigger a bailout action.
> +     * tests/tap-bailout-leading-space.test: New test.
> +     * tests/list-of-tests.mk: Add it.
> +     Problem reported by Jim Meyering in automake bug#10374.
> +
> +2011-12-27  Stefano Lattarini  <address@hidden>
> +
>       tests: fix spurious failure of cond29.test
>       * tests/cond29.test: Limit the amount of virtual memory available
>       to the automake process to ~ 50 MB, rather than only ~ 20 MB, to
> diff --git a/lib/tap-driver.sh b/lib/tap-driver.sh
> index c911991..62a4dc6 100755
> --- a/lib/tap-driver.sh
> +++ b/lib/tap-driver.sh
> @@ -573,12 +573,16 @@ while (1)
>          handle_tap_plan(0, $0)
>        }
>      # "Bail out!" magic.
> -    else if ($0 ~ /^Bail out!/)
> +    # Older versions of prove and TAP::Harness (e.g., 3.17) did not
> +    # recognize a "Bail out!" directive if that was preceded by leading

s/if that was/when/

> +    # whitespace, but more modern versions (e.g., 3.23) do.  So we try

s/ try//

> +    # emulate the latter, "more modern" behaviour.
> +    else if ($0 ~ /^[ \t]*Bail out!/)
>        {
>          bailed_out = 1
>          # Get the bailout message (if any), with leading and trailing
>          # whitespace stripped.  The message remains stored in `$0`.
> -        sub("^Bail out![ \t]*", "");
> +        sub("^[ \t]*Bail out![ \t]*", "");
>          sub("[ \t]*$", "");
>          # Format the error message for the
>          bailout_message = "Bail out!"
> diff --git a/tests/list-of-tests.mk b/tests/list-of-tests.mk
> index 71a7693..259b232 100644
> --- a/tests/list-of-tests.mk
> +++ b/tests/list-of-tests.mk
> @@ -1001,6 +1001,7 @@ syntax.test \
>  tap-ambiguous-directive.test \
>  tap-autonumber.test \
>  tap-bailout.test \
> +tap-bailout-leading-space.test \
>  tap-bailout-and-logging.test \
>  tap-bailout-suppress-badexit.test \
>  tap-bailout-suppress-later-diagnostic.test \
> diff --git a/tests/tap-bailout-leading-space.test 
> b/tests/tap-bailout-leading-space.test
> new file mode 100755
> index 0000000..9a89fee
> --- /dev/null
> +++ b/tests/tap-bailout-leading-space.test
> @@ -0,0 +1,72 @@
> +#! /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 <http://www.gnu.org/licenses/>.
> +
> +# Older versions of prove and TAP::Harness (e.g., 3.17) didn't recognize
> +# a "Bail out!" directive if that was preceded by leading whitespace,

s/if // or s/if that was/when/
s/leading //

General note:  This comment appears in three places.
IMHO, that feels like too many.

> +# but more modern versions (e.g., 3.23) do.  So we let this behaviour
> +# undefined for the perl implementation of the Automake TAP driver, but
> +# expect the latter, "more modern" behaviour in our awk TAP driver.
> +
> +am_parallel_tests=yes
> +am_tap_implementation=shell
> +. ./defs || Exit 1
> +
> +. "$testsrcdir"/tap-setup.sh || fatal_ "sourcing tap-setup.sh"
> +
> +cat > a.test <<END
> +1..1
> +ok 1
> + Bail out!
> +END
> +
> +cat > b.test <<END
> +1..1
> +ok 1 # SKIP
> +${tab}Bail out!
> +END
> +
> +cat > c.test <<END
> +1..1
> +  ${tab}  ${tab}${tab}Bail out!   FUBAR! $tab
> +END
> +
> +cat >> exp <<END
> +PASS: a.test 1
> +ERROR: a.test - Bail out!
> +SKIP: b.test 1
> +ERROR: b.test - Bail out!
> +ERROR: c.test - Bail out! FUBAR!
> +END
> +
> +TESTS='a.test b.test c.test' $MAKE -e check >stdout \
> +  && { cat stdout; Exit 1; }
> +cat stdout
> +
> +count_test_results total=5 pass=1 fail=0 xpass=0 xfail=0 skip=1 error=3
> +
> +LC_ALL=C sort exp > t
> +mv -f t exp
> +
> +# We need the sort below to account for parallel make usage.
> +grep ': [abcde]\.test' stdout \
> +  | sed "s/[ $tab]*#[ $tab]*SKIP.*//" \
> +  | LC_ALL=C sort > got
> +
> +cat exp
> +cat got
> +diff exp got
> +
> +:
> diff --git a/tests/tap-no-spurious.test b/tests/tap-no-spurious.test
> index bf3d7ae..647589a 100755
> --- a/tests/tap-no-spurious.test
> +++ b/tests/tap-no-spurious.test
> @@ -84,6 +84,10 @@ set -x # Reset shell xtraces.
>
>  # The prove(1) utility doesn't bail out on these, so our driver
>  # shouldn't either.
> +# Note that older versions of prove and TAP::Harness (e.g., 3.17) didn't
> +# recognize a "Bail out!" directive id preceded by leading white spaces,

s/id //
s/spaces/space/
s/leading //

> +# but more modern versions (e.g., 3.23) do.  So we let that behaviour

s/let/leave/

> +# undefined here; it should be checked in `tap-bailout-leading-space.test'.

s/should be/is/

>  cat >> all.test <<'END'
>  bailout
>  bailout!
> @@ -93,7 +97,6 @@ Bailout
>  Bailout!
>  Bail out
>   Bail out
> - Bail out!
>  #Bail out!
>  # Bail out!
>  END