[FYI] {test-protocols} tap: plan location is more liberal w.r.t. non-TAP lines

[Stefano Lattarini]

With this change, only lines that are TAP results will matter
w.r.t. the position of the TAP plan in the input; for example,
this input:
  this is a non-TAP line
  # and this a TAP diagnostic line
  1..1
  ok 1
was considered to be an error, diagnosed with a message "test
plan in middle of output"; as effect of the current change, such
input is now valid.  This is more consistent with the behaviour
of the `prove' utility.

* lib/tap-driver ($lineno): Removed, no more needed.
($tap_stopped): New global variable.
(stringify_test_result): Return "ERROR" if a TAP result is found
when `$tap_stopped' is set to true.
(handle_tap_test): Diagnose TAP results that comes after a late
plan.  Add a couple of blank lines, for clarity.
(handle_tap_plan): Set `$tap_stopped' to true after a late plan
is encountered.  Do not complain anymore for extra non-TAP lines
preceding or following the plan.  Adjust comments.
(main): Don't increment $lineno anymore.
* tests/tap-plan.test: Extend a bit, and remove stale comment.
* tests/tap-color.test: Adjust.
* tests/tap-passthrough.test: Likewise.
* tests/tap-plan-corner.test: Adjust and extend.
* tests/tap-plan-errors.test: Likewise.
* tests/tap-plan-middle.test: New test.
* tests/tap-plan-corner2.test: Delete, it's obsolete now.
* tests/Makefile.am (XFAIL_TESTS): Remove it.
(tap_with_common_setup_tests): Likewise, and add
`tap-plan-corner.test'.
---
 ChangeLog                                          |   35 ++++++++
 lib/tap-driver                                     |   27 ++++--
 tests/Makefile.am                                  |    3 +-
 tests/Makefile.in                                  |    6 +-
 tests/tap-color.test                               |    8 +-
 tests/tap-passthrough.test                         |    2 +-
 tests/tap-plan-corner.test                         |   89 ++++++++++++++++---
 tests/tap-plan-errors.test                         |   31 ++++----
 ...{tap-plan-corner2.test => tap-plan-middle.test} |   56 ++++++++++---
 tests/tap-plan.test                                |   11 ++-
 10 files changed, 204 insertions(+), 64 deletions(-)
 rename tests/{tap-plan-corner2.test => tap-plan-middle.test} (51%)

diff --git a/ChangeLog b/ChangeLog
index 9a579b8..33ac71c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,40 @@
 2011-08-07  Stefano Lattarini  <address@hidden>
 
+       tap: plan location is more liberal w.r.t. non-TAP lines
+       With this change, only lines that are TAP results will matter
+       w.r.t. the position of the TAP plan in the input; for example,
+       this input:
+         this is a non-TAP line
+         # and this a TAP diagnostic line
+         1..1
+         ok 1
+       was considered to be an error, diagnosed with a message "test
+       plan in middle of output"; as effect of the current change, such
+       input is now valid.  This is more consistent with the behaviour
+       of the `prove' utility.
+       * lib/tap-driver ($lineno): Removed, no more needed.
+       ($tap_stopped): New global variable.
+       (stringify_test_result): Return "ERROR" if a TAP result is found
+       when `$tap_stopped' is set to true.
+       (handle_tap_test): Diagnose TAP results that comes after a late
+       plan.  Add a couple of blank lines, for clarity.
+       (handle_tap_plan): Set `$tap_stopped' to true after a late plan
+       is encountered.  Do not complain anymore for extra non-TAP lines
+       preceding or following the plan.  Adjust comments.
+       (main): Don't increment $lineno anymore.
+       * tests/tap-plan.test: Extend a bit, and remove stale comment.
+       * tests/tap-color.test: Adjust.
+       * tests/tap-passthrough.test: Likewise.
+       * tests/tap-plan-corner.test: Adjust and extend.
+       * tests/tap-plan-errors.test: Likewise.
+       * tests/tap-plan-middle.test: New test.
+       * tests/tap-plan-corner2.test: Delete, it's obsolete now.
+       * tests/Makefile.am (XFAIL_TESTS): Remove it.
+       (tap_with_common_setup_tests): Likewise, and add
+       `tap-plan-corner.test'.
+
+2011-08-07  Stefano Lattarini  <address@hidden>
+
        testsuite: remove now-passing test from XFAIL_TESTS
        * tests/Makefile.am (XFAIL_TESTS): Remove `tap-signal.test',
        which is passing since previous commit `v1.11-974-gc7fa872'.
diff --git a/lib/tap-driver b/lib/tap-driver
index 1f5a271..948c3d5 100755
--- a/lib/tap-driver
+++ b/lib/tap-driver
@@ -43,11 +43,14 @@ my %COLOR = (
 #  Global variables.  #
 # ------------------- #
 
-my $lineno = 0;     # Number of input lines seen so far.
 my $testno = 0;     # Number of test results seen so far.
 my $plan_seen = 0;  # Whether the TAP plan has been seen or not.
 my $parser;         # TAP parser object (will be initialized later).
 
+# When true, it means that the rest of the input stream cannot
+# contain any further TAP results.
+my $tap_stopped = 0;
+
 # ----------------- #
 #  Option parsing.  #
 # ----------------- #
@@ -272,7 +275,7 @@ sub stringify_test_result ($)
   my $result = shift;
   my $PASS = $cfg{"expect-failure"} ? "XPASS": "PASS";
   my $FAIL = $cfg{"expect-failure"} ? "XFAIL": "FAIL";
-  if ($result->is_unplanned || $result->number != $testno)
+  if ($result->is_unplanned || $result->number != $testno || $tap_stopped)
     {
       return "ERROR";
     }
@@ -362,7 +365,12 @@ sub handle_tap_test ($)
     {
       $string .= " $description";
     }
-  if ($test->is_unplanned)
+
+  if ($tap_stopped)
+    {
+      $string .= " # AFTER LATE PLAN";
+    }
+  elsif ($test->is_unplanned)
     {
       $string .= " # UNPLANNED";
     }
@@ -378,20 +386,20 @@ sub handle_tap_test ($)
           $string .= " $explanation";
         }
     }
+
   report $test_result, $string;
 }
 
 sub handle_tap_plan ($)
 {
   my $plan = shift;
+  # Only one plan per stream is acceptable.
   testsuite_error "multiple test plans" if $plan_seen;
   $plan_seen = 1;
-  # TAP plan must be either in the first or in the last line.
-  if ($lineno > 1 && peek_tap_line)
-    {
-      testsuite_error "test plan in middle of output";
-      return;
-    }
+  # TAP plan must come either before or after *all* the TAP results.
+  # So, if we find it after having already seen at least one TAP result,
+  # set a flag signaling that no more TAP results are acceptable.
+  $tap_stopped = 1 if $testno >= 1;
   # Nothing more to do, unless the plan contains a SKIP directive.
   return
     if not defined $plan->directive && length ($plan->directive) > 0;
@@ -428,7 +436,6 @@ sub main (@)
     {
       # Verbatim copy any input line into the log file.
       print $cur->raw . "\n";
-      $lineno++;
       if ($cur->is_plan)
         {
           handle_tap_plan ($cur);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fb85ef..43c11d1 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-plan-corner2.test \
 tap-message-0.test \
 txinfo5.test
 
@@ -1169,8 +1168,8 @@ tap-passthrough.test \
 tap-passthrough-exit.test \
 tap-plan.test \
 tap-plan-corner.test \
-tap-plan-corner2.test \
 tap-plan-errors.test \
+tap-plan-middle.test \
 tap-realtime.test \
 tap-recheck-logs.test \
 tap-skip-whole-whitespace.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 15cf791..d98f473 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -285,8 +285,8 @@ 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-plan-corner2.test \
-       tap-message-0.test txinfo5.test $(instspc_xfail_tests)
+       yacc-dist-nobuild-subdir.test tap-message-0.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 \
        check-p.test check11-p.test check12-p.test check2-p.test \
@@ -1405,8 +1405,8 @@ tap-passthrough.test \
 tap-passthrough-exit.test \
 tap-plan.test \
 tap-plan-corner.test \
-tap-plan-corner2.test \
 tap-plan-errors.test \
+tap-plan-middle.test \
 tap-realtime.test \
 tap-recheck-logs.test \
 tap-skip-whole-whitespace.test \
diff --git a/tests/tap-color.test b/tests/tap-color.test
index dbd6629..cd0329b 100755
--- a/tests/tap-color.test
+++ b/tests/tap-color.test
@@ -70,9 +70,10 @@ Bail out!
 END
 
 cat > badplan.test << 'END'
-# foo
-1..1
+1..2
 ok 1
+1..2
+ok 2
 END
 
 cat > noplan.test << 'END'
@@ -109,7 +110,8 @@ test_color ()
   cat stdout | grep "^${blu}SKIP${std}: skip\.test - whole script$"
   cat stdout | grep "^${grn}PASS${std}: bail\.test 1$"
   cat stdout | grep "^${mgn}ERROR${std}: bail\.test - Bail out!$"
-  cat stdout | grep "^${mgn}ERROR${std}: badplan\.test - test plan in middle 
of output$"
+  cat stdout | grep "^${mgn}ERROR${std}: badplan\.test - multiple test plans$"
+  cat stdout | grep "^${mgn}ERROR${std}: badplan\.test 2 # AFTER LATE PLAN$"
   cat stdout | grep "^${mgn}ERROR${std}: noplan\.test - missing test plan$"
   cat stdout | grep "^${mgn}ERROR${std}: few.test - too few tests run 
(expected 2, got 1)$"
   cat stdout | grep "^${mgn}ERROR${std}: many.test - too many tests run 
(expected 1, got 2)$"
diff --git a/tests/tap-passthrough.test b/tests/tap-passthrough.test
index ba773ed..6406548 100755
--- a/tests/tap-passthrough.test
+++ b/tests/tap-passthrough.test
@@ -154,7 +154,7 @@ for rx in \
   '^ok 23$' \
   '^Misplaced plan$' \
   '^1\.\.13$' \
-  '^ERROR:.* test plan in middle of output' \
+  '^ERROR:.* multiple test plans' \
    '^Extra test$' \
   '^Last line$' \
   '^ERROR:.* [tT]oo many tests run.*expected 3, got 4' \
diff --git a/tests/tap-plan-corner.test b/tests/tap-plan-corner.test
index 4215556..eaebcf2 100755
--- a/tests/tap-plan-corner.test
+++ b/tests/tap-plan-corner.test
@@ -22,42 +22,103 @@ parallel_tests=yes
 
 . "$testsrcdir"/tap-setup.sh || fatal_ "sourcing tap-setup.sh"
 
-cat > leading-repeated-1.test <<END
+cat > leading-repeated.test <<END
 1..1
 1..1
 ok 1
 END
 
-cat > trailing-repeated-1.test <<END
+cat > trailing-repeated.test <<END
 ok 1
 1..1
 1..1
 END
 
-cat > leading-repeated-2.test <<END
+for pos in leading trailing; do
+  TESTS="$pos-repeated.test" $MAKE -e check >stdout \
+    && { cat stdout; Exit 1; }
+  cat stdout
+  count_test_results total=2 pass=1 fail=0 xpass=0 xfail=0 skip=0 error=1
+  grep "^ERROR: $pos-repeated\\.test - multiple test plans$" stdout
+done
+
+cat > leading-repeated.test <<END
 1..2
 ok 1
 1..2
 ok 2
 END
 
-cat > trailing-repeated-2.test <<END
+cat > trailing-repeated.test <<END
 ok 1
 1..2
 ok 2
 1..2
 END
 
-for i in 1 2; do
-  for kind in leading trailing; do
-    cp $kind-repeated-$i.test all.test
-    $MAKE check >stdout && { cat stdout; Exit 1; }
-    cat stdout
-    count_test_results total=`expr $i + 2` pass=$i fail=0 \
-                       xpass=0 xfail=0 skip=0 error=2
-    grep '^ERROR: all\.test - test plan in middle of output$' stdout
-    grep '^ERROR: all\.test - multiple test plans$' stdout
-  done
+env TESTS="leading-repeated.test trailing-repeated.test" \
+  $MAKE -e check >stdout && { cat stdout; Exit 1; }
+cat stdout
+count_test_results total=6 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=4
+for pos in leading trailing; do
+  grep "^ERROR: $pos-repeated\\.test - multiple test plans$" stdout
+  grep "^ERROR: $pos-repeated\\.test 2 # AFTER LATE PLAN$" stdout
 done
 
+cat > all.test <<END
+1..5
+ok 1
+ok 2
+1..5
+ok 3
+1..5
+ok 4
+1..5
+ok 5
+END
+
+$MAKE -e check >stdout && { cat stdout; Exit 1; }
+cat stdout
+count_test_results total=8 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=6
+
+cat > exp <<'END'
+PASS: all.test 1
+PASS: all.test 2
+ERROR: all.test - multiple test plans
+ERROR: all.test 3 # AFTER LATE PLAN
+ERROR: all.test - multiple test plans
+ERROR: all.test 4 # AFTER LATE PLAN
+ERROR: all.test - multiple test plans
+ERROR: all.test 5 # AFTER LATE PLAN
+END
+
+$FGREP ': all.test' stdout > got
+
+cat exp
+cat got
+diff exp got
+
+sed -e 1d all.test > t
+mv -f t all.test
+
+$MAKE -e check >stdout && { cat stdout; Exit 1; }
+cat stdout
+count_test_results total=7 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=5
+
+cat > exp <<'END'
+PASS: all.test 1
+PASS: all.test 2
+ERROR: all.test 3 # AFTER LATE PLAN
+ERROR: all.test - multiple test plans
+ERROR: all.test 4 # AFTER LATE PLAN
+ERROR: all.test - multiple test plans
+ERROR: all.test 5 # AFTER LATE PLAN
+END
+
+$FGREP ': all.test' stdout > got
+
+cat exp
+cat got
+diff exp got
+
 :
diff --git a/tests/tap-plan-errors.test b/tests/tap-plan-errors.test
index fd12e1b..95cc640 100755
--- a/tests/tap-plan-errors.test
+++ b/tests/tap-plan-errors.test
@@ -16,9 +16,9 @@
 
 # TAP support: the following situations should be flagged as errors:
 #  - unmatched test plan (too few tests run)
-#  - misplaced test plan
 #  - multiple test plans
 #  - missing test plan
+#  - misplaced test plan (tests run after a late plan)
 # Checks about unplanned tests are performed in 'tap-unplanned.test'.
 # More checks about corner-cases in TAP plans are performed in
 # 'tap-plan-corner.test' and 'tap-plan-corner2.test'.
@@ -36,17 +36,17 @@ my_check ()
   $MAKE check >stdout && { cat stdout; Exit 1; }
   cat stdout
   count_test_results "$@"
-  grep "^ERROR: all\\.test - $err$" stdout
+  grep "^ERROR: all\\.test $err$" stdout
   unset err
 }
 
-err='too few tests run (expected 2, got 1)'
+err='- too few tests run (expected 2, got 1)'
 my_check total=2 pass=1 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
 1..2
 ok 1
 END
 
-err='too few tests run (expected 12, got 3)'
+err='- too few tests run (expected 12, got 3)'
 my_check total=4 pass=2 fail=0 xpass=0 xfail=1 skip=0 error=1 <<END
 ok 1
 ok 2
@@ -54,29 +54,30 @@ not ok 3 # TODO
 1..12
 END
 
-err='too few tests run (expected 1, got 0)'
+err='- too few tests run (expected 1, got 0)'
 my_check total=1 pass=0 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
 1..1
 END
 
-err='test plan in middle of output'
-my_check total=3 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
-oops
-1..2
+err='2 # AFTER LATE PLAN'
+my_check total=2 pass=1 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
 ok 1
+1..2
 ok 2
 END
 
-err='test plan in middle of output'
-my_check total=3 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
+err='5 # AFTER LATE PLAN'
+my_check total=5 pass=4 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
 ok 1
 ok 2
-1..2
-oops
+ok 3
+ok 4
+1..5
+ok 5
 END
 
 # The two test plans here are deliberately equal.
-err='multiple test plans'
+err='- multiple test plans'
 my_check total=3 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
 1..2
 ok 1
@@ -84,7 +85,7 @@ ok 2
 1..2
 END
 
-err='missing test plan'
+err='- missing test plan'
 my_check total=2 pass=1 fail=0 xpass=0 xfail=0 skip=0 error=1 <<END
 ok 1
 END
diff --git a/tests/tap-plan-corner2.test b/tests/tap-plan-middle.test
similarity index 51%
rename from tests/tap-plan-corner2.test
rename to tests/tap-plan-middle.test
index 54fd5e9..88bb647 100755
--- a/tests/tap-plan-corner2.test
+++ b/tests/tap-plan-middle.test
@@ -15,35 +15,69 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # TAP support:
-#  - more corner cases for TAP plan
+#  - test plan preceding and/or following non-result TAP lines
 
 parallel_tests=yes
 . ./defs || Exit 1
 
 . "$testsrcdir"/tap-setup.sh || fatal_ "sourcing tap-setup.sh"
 
-# The leading blank line is meant.
-cat > leading-blank.test <<END
+cat > top1.test <<END
+non-TAP line, ignored
+1..1
+ok 1
+END
 
+cat > top2.test <<END
+some
+non-TAP
+lines
+are
+ignored
+# and a TAP comment won't cause problems either
 1..2
 ok 1
 ok 2
 END
 
-# The trailing blank line is meant.
-cat > trailing-blank.test <<END
+# Try with a blank line too, just to be sure.
+cat > top3.test <<END
+
+1..1
+ok 1
+END
+
+cat > bot1.test <<END
+ok 1 # SKIP
+1..1
+bla blah blah ...
+END
+
+cat > bot2.test <<END
 ok 1
 ok 2
-1..2
+not ok 3 # TODO
+1..3
address@hidden (a cursing comment :-)
+END
+
+# Try with a blank line too, just to be sure.
+cat > bot3.test <<END
+ok 1
+not ok 2 # TODO
+ok 3 # SKIP
+ok 4 # SKIP
+1..4
 
 END
 
-for kind in leading trailing; do
-  cp $kind-blank.test all.test
-  $MAKE check >stdout && { cat stdout; Exit 1; }
+tests=`echo *.test`
+
+for tap_flags in "" "--comments"; do
+  env TEST_LOG_DRIVER_FLAGS="$tap_flags" TESTS="$tests" \
+    $MAKE -e check >stdout || { cat stdout; Exit 1; }
   cat stdout
-  count_test_results total=3 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=1
-  grep '^ERROR: all\.test - test plan in middle of output$' stdout
+  count_test_results total=12 pass=7 xfail=2 skip=3 fail=0 xpass=0 error=0
 done
 
 :
diff --git a/tests/tap-plan.test b/tests/tap-plan.test
index 79e9bdd..e2fda35 100755
--- a/tests/tap-plan.test
+++ b/tests/tap-plan.test
@@ -17,7 +17,6 @@
 # TAP support:
 #  - test scripts with the test plan at the beginning
 #  - test scripts with the test plan at the end
-#  - test scripts without a test plan
 
 parallel_tests=yes
 . ./defs || Exit 1
@@ -42,10 +41,12 @@ ok
 1..4
 END
 
-# Check that the plans doesn't cause any problem
+for tap_flags in "" "--comments"; do
+  env TEST_LOG_DRIVER_FLAGS="$tap_flags" TESTS='top.test bot.test' \
+    $MAKE -e check >stdout || { cat stdout; Exit 1; }
+  cat stdout
+  count_test_results total=7 pass=5 xfail=1 skip=1 fail=0 xpass=0 error=0
+done
 
-TESTS='top.test bot.test' $MAKE -e check >stdout || { cat stdout; Exit 1; }
-cat stdout
-count_test_results total=7 pass=5 xfail=1 skip=1 fail=0 xpass=0 error=0
 
 :
-- 
1.7.2.3