[FYI] {test-protocols} tap: new option to change the string designating TAP diagnostic

[Stefano Lattarini]

Here we introduce a new option `--diagnostic-string' in our TAP
test driver, that allows the user to specify which string should
denote the beginning of a TAP diagnostic line.  This change is
not gratuitous, nor result if over-engineering: it is motivated
by real issues that have emerged during the use of TAP in the
Automake's own testsuite (see the commit `v1.11-1082-g9b967c2'
"testsuite: yet more use of TAP, and related extensions").

* doc/automake.texi (Use TAP with Automake test harness): Document
the new option.
(Incompatibilities with other TAP parsers and drivers): Report it
as a potential source of incompatibility.
* lib/tap-driver ($diag_string): New global variable, defaulting
to "#", and whose value can be changed ...
(Getopt::Long::GetOptions): ... by the newly recognized option
`--diagnostic-string'.
(handle_tap_comment): Subroutine removed, some of its simple logic
inlined ...
(main): ... in here, where now ...
(extract_tap_comment): ... this new subroutine is used.
($USAGE): Adjust.
* tests/tap-diagnostic.test: Make one check slightly stricter.
* tests/tap-diag-custom.test: New test.
* tests/Makefile.am (tap_other_tests): Add it.
---
 ChangeLog                        |   27 +++++++++++
 doc/automake.texi                |   17 +++++++
 lib/tap-driver                   |   28 +++++++----
 tests/Makefile.am                |    1 +
 tests/Makefile.in                |    1 +
 tests/tap-diagnostic-custom.test |   96 ++++++++++++++++++++++++++++++++++++++
 tests/tap-diagnostic.test        |    2 +-
 7 files changed, 161 insertions(+), 11 deletions(-)
 create mode 100755 tests/tap-diagnostic-custom.test

diff --git a/ChangeLog b/ChangeLog
index c023265..80e5163 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,32 @@
 2011-08-05  Stefano Lattarini  <address@hidden>
 
+       tap: new option to change the string designating TAP diagnostic
+       Here we introduce a new option `--diagnostic-string' in our TAP
+       test driver, that allows the user to specify which string should
+       denote the beginning of a TAP diagnostic line.  This change is
+       not gratuitous, nor result if over-engineering: it is motivated
+       by real issues that have emerged during the use of TAP in the
+       Automake's own testsuite (see the commit `v1.11-1082-g9b967c2'
+       "testsuite: yet more use of TAP, and related extensions").
+       * doc/automake.texi (Use TAP with Automake test harness): Document
+       the new option.
+       (Incompatibilities with other TAP parsers and drivers): Report it
+       as a potential source of incompatibility.
+       * lib/tap-driver ($diag_string): New global variable, defaulting
+       to "#", and whose value can be changed ...
+       (Getopt::Long::GetOptions): ... by the newly recognized option
+       `--diagnostic-string'.
+       (handle_tap_comment): Subroutine removed, some of its simple logic
+       inlined ...
+       (main): ... in here, where now ...
+       (extract_tap_comment): ... this new subroutine is used.
+       ($USAGE): Adjust.
+       * tests/tap-diagnostic.test: Make one check slightly stricter.
+       * tests/tap-diag-custom.test: New test.
+       * tests/Makefile.am (tap_other_tests): Add it.
+
+2011-08-05  Stefano Lattarini  <address@hidden>
+
        check: add small "synchronization" comment to `test-driver'
        * lib/test-driver: Add comment to the code initializing ANSI color
        escapes, telling to keep it in sync with the similar initialization
diff --git a/doc/automake.texi b/doc/automake.texi
index cab0fca..254e98c 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -9667,6 +9667,17 @@ harness to get confused if anything that appears on 
standard error
 looks like a test result.
 @item --no-merge
 Revert the effects of @option{--merge}.
address@hidden address@hidden
+Change the string that introduces TAP diagnostic from the default value
+of address@hidden'' to @address@hidden  This can be useful if your
+TAP-based test scripts produce verbose output on which they have limited
+control (because, say, the output comes by other tools invoked in the
+scripts), and it might contain text that gets spuriously interpreted as
+TAP diagnostic: such an issue can be solved by redefining the string that
+activates TAP diagnostic to a value you know won't appear by chance in
+the tests' output.  Note however that this feature is non-standard, as
+the ``official'' TAP protocol does not allow for such a customization; so
+don't use it if you can avoid it.
 @end table
 
 @noindent
@@ -9761,6 +9772,12 @@ the test script it occurs into.  This doesn't follows 
TAP specifications,
 but on the other hand it maximizes compatibility (and code sharing) with
 the ``hard error'' concept of the default @option{parallel-tests} driver.
 @item
+The @option{--diagnostic-string} option of out driver allows to modify
+the string that introduces TAP diagnostic from the default value
+of address@hidden''.  The standard TAP protocol has currently no way to
+allow this, so if you use it your diagnostic will be lost to more
+compliant tools like @command{prove} and @code{Test::Harness}
address@hidden
 @emph{TODO}: there's surely something else ...
 @end itemize
 
diff --git a/lib/tap-driver b/lib/tap-driver
index b0c61e5..c748c5c 100755
--- a/lib/tap-driver
+++ b/lib/tap-driver
@@ -17,8 +17,9 @@ my $USAGE = <<'END';
 Usage:
   tap-driver --test-name=NAME --log-file=PATH --trs-file=PATH
              [--expect-failure={yes|no}] [--color-tests={yes|no}]
-             [--enable-hard-errors={yes|no}] [--merge|--no-merge]
-             [--ignore-exit] [--comments|--no-comments] [--] TEST-COMMAND
+             [--enable-hard-errors={yes|no}] [--ignore-exit]
+             [--diagnostic-string=STRING] [--merge|--no-merge]
+             [--comments|--no-comments] [--] TEST-COMMAND
 The `--test-name' and `--log-file' options are mandatory.
 END
 
@@ -63,6 +64,7 @@ my %cfg = (
 my $test_script_name = undef;
 my $log_file = undef;
 my $trs_file = undef;
+my $diag_string = "#";
 
 Getopt::Long::GetOptions (
     'help' => sub { print $HELP; exit 0; },
@@ -73,6 +75,7 @@ Getopt::Long::GetOptions (
     'color-tests=s'  => \&bool_opt,
     'expect-failure=s'  => \&bool_opt,
     'enable-hard-errors=s' => \&bool_opt,
+    'diagnostic-string=s' => \$diag_string,
     'comments' => sub { $cfg{"comments"} = 1; },
     'no-comments' => sub { $cfg{"comments"} = 0; },
     'merge' => sub { $cfg{"merge"} = 1; },
@@ -348,11 +351,15 @@ sub handle_tap_bailout ($)
   finish;
 }
 
-sub handle_tap_comment ($)
+sub extract_tap_comment ($)
 {
-  return unless $cfg{comments};
-  my $comment = $_[0]->comment;
-  report "#", "$comment" if length $comment;
+  local $_ = shift;
+  if (/^\Q$diag_string\E(.*)$/o)
+    {
+      (my $comment = $1) =~ s/(?:^\s*|\s*$)//;
+      return $comment;
+    }
+  return "";
 }
 
 sub main (@)
@@ -372,14 +379,15 @@ sub main (@)
         {
           handle_tap_test ($cur);
         }
-      elsif ($cur->is_comment)
-        {
-          handle_tap_comment ($cur);
-        }
       elsif ($cur->is_bailout)
         {
           handle_tap_bailout ($cur);
         }
+      elsif ($cfg{comments})
+        {
+          my $comment = extract_tap_comment ($cur->raw);
+          report "#", "$comment" if length $comment;
+       }
     }
   if (!$plan_seen)
     {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9fdb891..7b3b777 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1197,6 +1197,7 @@ tap_other_tests = \
 tap-common-setup.test \
 tap-bad-prog.tap \
 tap-basic.test \
+tap-diagnostic-custom.test \
 tap-doc.test \
 tap-doc2.test \
 tap-more.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index a3a356a..f03ec16 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -1430,6 +1430,7 @@ tap_other_tests = \
 tap-common-setup.test \
 tap-bad-prog.tap \
 tap-basic.test \
+tap-diagnostic-custom.test \
 tap-doc.test \
 tap-doc2.test \
 tap-more.test \
diff --git a/tests/tap-diagnostic-custom.test b/tests/tap-diagnostic-custom.test
new file mode 100755
index 0000000..d965229
--- /dev/null
+++ b/tests/tap-diagnostic-custom.test
@@ -0,0 +1,96 @@
+#! /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/>.
+
+# TAP support:
+#  - option '--diagnostic-string' to customize the string introducing
+#    TAP diagnostics
+
+parallel_tests=yes
+. ./defs || Exit 1
+
+cp "$top_testsrcdir"/lib/tap-driver . \
+  || fatal_ "failed to fetch auxiliary script tap-driver"
+
+cat >> configure.in <<END
+AC_SUBST([PERL], ['$PERL'])
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+my_log_driver = $(PERL) $(srcdir)/tap-driver
+my_log_compiler = cat
+TEST_EXTENSIONS =
+TESTS =
+END
+
+: > later.mk
+
+# Quoting our comments below is an hack required to keep
+# comments near the things they refer to.
+i=0
+for string in \
+'## A letter' \
+    a \
+'## A number' \
+    1023 \
+'## A non-alphabetic character' \
+    @ \
+'## Some metacharacters (we need to repeat the "$" for make)' \
+    '^>;&*"|$$' \
+'## A whitespace character' \
+    " " \
+'## A tab character' \
+    "$tab" \
+'## A string with more whitespace' \
+    "  ${tab}a b${tab} c" \
+'## Note the we do not have the empty string here.  We prefer to' \
+'## leave its behaviour in this context undefined for the moment.'
+do
+  case $string in '##'*) continue;; esac
+  i=`expr $i + 1`
+  unindent >> Makefile.am << END
+    TEST_EXTENSIONS += .t$i
+    TESTS += foo$i.t$i
+    T${i}_LOG_COMPILER = \$(my_log_compiler)
+    T${i}_LOG_DRIVER = \$(my_log_driver)
+    AM_T${i}_LOG_DRIVER_FLAGS = \
+      --comments \
+      --diagnostic-string '$string'
+END
+  unindent > foo$i.t$i <<END
+    1..1
+    ok 1
+    $string blah blah $i
+END
+    echo "AM_T${i}_LOG_DRIVER_FLAGS = --no-comments" >> later.mk
+done
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+
+$MAKE check >stdout || { cat stdout; Exit 1; }
+cat stdout
+count_test_results total=$i pass=$i fail=0 xpass=0 xfail=0 skip=0 error=0
+
+cat later.mk >> Makefile
+$MAKE check >stdout || { cat stdout; Exit 1; }
+cat stdout
+$FGREP 'blah blah' stdout && Exit 1
+
+:
diff --git a/tests/tap-diagnostic.test b/tests/tap-diagnostic.test
index 73ab45e..3c7da79 100755
--- a/tests/tap-diagnostic.test
+++ b/tests/tap-diagnostic.test
@@ -117,7 +117,7 @@ count_test_results total=1 pass=1 fail=0 xpass=0 xfail=0 
skip=0 error=0
 grep "^# all.test:${ws0p}foo$" stdout
 grep "^# all.test:${ws0p}bar${ws0p}$" stdout
 grep "^# all.test:${ws1p}zardoz${ws0p}$" stdout
-grep "^# all.test:${ws1p}foo${ws1p}bar${ws1p}baz${ws0p}$" stdout
+grep "^# all.test:${ws1p}foo  bar${tab}baz${ws0p}$" stdout
 
 test `grep -c '^# all\.test:' stdout` -eq 4
 
-- 
1.7.2.3