[FYI 1/2] {test-protocols} tap: correctly handle string "0" in TAP messages

[Stefano Lattarini]

* lib/tap-driver.pl (is_null_string): New function, can be used
to determine whether a given string variable is empty or undefined.
Useful to avoid pitfalls like:
  if ($message) { print "$message\n"; }
which wouldn't print anything if $message is the literal "0".
(handle_tap_test, handle_tap_plan, handle_tap_bailout): Use it,
to avoid missing messages composed only by a literal "0" in TAP
result descriptions and in skip, todo and bailout explanations.
* tests/tap-message-0.test: Enhance.
* tests/Makefile.am (XFAIL_TESTS): Remove it, it passes now.
---
 ChangeLog                |   14 ++++++++++++++
 lib/tap-driver.pl        |   32 +++++++++++++++++++++-----------
 tests/Makefile.am        |    1 -
 tests/Makefile.in        |    2 +-
 tests/tap-message-0.test |   23 +++++++++++++++++++----
 5 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 571218b..985cc86 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2011-08-17  Stefano Lattarini  <address@hidden>
 
+       tap: correctly handle string "0" in TAP messages
+       * lib/tap-driver.pl (is_null_string): New function, can be used
+       to determine whether a given string variable is empty or undefined.
+       Useful to avoid pitfalls like:
+         if ($message) { print "$message\n"; }
+       which wouldn't print anything if $message is the literal "0".
+       (handle_tap_test, handle_tap_plan, handle_tap_bailout): Use it,
+       to avoid missing messages composed only by a literal "0" in TAP
+       result descriptions and in skip, todo and bailout explanations.
+       * tests/tap-message-0.test: Enhance.
+       * tests/Makefile.am (XFAIL_TESTS): Remove it, it passes now.
+
+2011-08-17  Stefano Lattarini  <address@hidden>
+
        tap: a minor simplification in the perl TAP driver
        * lib/tap-driver.pl: The `--disable-hard-errors' option is a
        no-op, so just ignore it and its argument.
diff --git a/lib/tap-driver.pl b/lib/tap-driver.pl
index ed99131..7043815 100755
--- a/lib/tap-driver.pl
+++ b/lib/tap-driver.pl
@@ -106,6 +106,7 @@ sub get_test_results ();
 sub handle_tap_bailout ($);
 sub handle_tap_plan ($);
 sub handle_tap_test ($);
+sub is_null_string ($);
 sub main (@);
 sub must_recheck ();
 sub report ($;$);
@@ -136,6 +137,16 @@ sub bool_opt ($$)
     }
 }
 
+# If the given string is undefined or empty, return true, otherwise
+# return false.  This function is useful to avoid pitfalls like:
+#   if ($message) { print "$message\n"; }
+# which wouldn't print anything if $message is the literal "0".
+sub is_null_string ($)
+{
+  my $str = shift;
+  return ! (defined $str and length $str);
+}
+
 # Convert a boolean to a "yes"/"no" string.
 sub yn ($)
 {
@@ -329,10 +340,9 @@ sub handle_tap_test ($)
   my $test_result = stringify_test_result $test;
   my $string = $test->number;
   
-  if (my $description = $test->description)
-    {
-      $string .= " $description";
-    }
+  my $description = $test->description;
+  $string .= " $description"
+    unless is_null_string $description;
 
   if ($plan_seen == LATE_PLAN)
     {
@@ -349,10 +359,9 @@ sub handle_tap_test ($)
   elsif (my $directive = $test->directive)
     {
       $string .= " # $directive";
-      if (my $explanation = $test->explanation)
-        {
-          $string .= " $explanation";
-        }
+      my $explanation = $test->explanation;
+      $string .= " $explanation"
+        unless is_null_string $explanation;
     }
 
   report $test_result, $string;
@@ -381,8 +390,8 @@ sub handle_tap_plan ($)
   # of SKIP result.
   if ($plan->directive && $testno == 0)
     {
-      my $explanation = $plan->explanation ?
-                        "- " . $plan->explanation : undef;
+      my $explanation = is_null_string ($plan->explanation) ?
+                        undef : "- " . $plan->explanation;
       report "SKIP", $explanation;
     }
 }
@@ -391,7 +400,8 @@ sub handle_tap_bailout ($)
 {
   my ($bailout, $msg) = ($_[0], "Bail out!");
   $bailed_out = 1;
-  $msg .= " " . $bailout->explanation if $bailout->explanation;
+  $msg .= " " . $bailout->explanation
+    unless is_null_string $bailout->explanation;
   testsuite_error $msg;
 }
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index be61a41..174f07b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -31,7 +31,6 @@ gcj6.test \
 override-conditional-2.test \
 pr8365-remake-timing.test \
 yacc-dist-nobuild-subdir.test \
-tap-message-0.test \
 txinfo5.test
 
 
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 5a17514..b014a8d 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -288,7 +288,7 @@ EXTRA_DIST = ChangeLog-old gen-parallel-tests 
instspc-tests.sh \
        extract-testsuite-summary tap-setup.sh tap-summary-aux.sh
 XFAIL_TESTS = all.test auxdir2.test cond17.test gcj6.test \
        override-conditional-2.test pr8365-remake-timing.test \
-       yacc-dist-nobuild-subdir.test tap-message-0.test txinfo5.test \
+       yacc-dist-nobuild-subdir.test txinfo5.test \
        $(instspc_xfail_tests)
 parallel_tests = backcompat5-p.test check-exported-srcdir-p.test \
        check-fd-redirect-p.test check-tests-in-builddir-p.test \
diff --git a/tests/tap-message-0.test b/tests/tap-message-0.test
index bce333f..87b2492 100755
--- a/tests/tap-message-0.test
+++ b/tests/tap-message-0.test
@@ -15,10 +15,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # TAP support:
-#  - having "0" as a test description (or TODO or SKIP message) should
-#    be supported
-# Note that a bug in some versions of TAP::Parser causes this not to be
-# generally true!
+#  - having "0" as a test description (or TODO or SKIP or "Bail out!"
+#    message) should be supported
 
 parallel_tests=yes
 . ./defs || Exit 1
@@ -63,4 +61,21 @@ cat exp
 cat got
 diff exp got
 
+echo '1..0 # SKIP 0' > all.test
+$MAKE check >stdout || { cat stdout; Exit 1; }
+cat stdout
+
+count_test_results total=1 pass=0 fail=0 xpass=0 xfail=0 skip=1 error=0
+
+grep '^SKIP: all.test - 0' stdout
+
+echo 'Bail out! 0' > all.test
+$MAKE check >stdout && { cat stdout; Exit 1; }
+cat stdout
+
+count_test_results total=1 pass=0 fail=0 xpass=0 xfail=0 skip=0 error=1
+
+grep '^ERROR: all.test - Bail out! 0' stdout
+
+
 :
-- 
1.7.2.3