Subject: [PATCH] tap: new `--merge-prefix' option to prefix stderr before merging
From: Richard Hansen <[email-masked]>
To: [email-masked]
Date: Fri, 9 Aug 2024 04:38:48 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Attached are a couple of patches to improve the TAP driver.  I don't know if `--merge-prefix' is the best name for the new option; suggestions for alternatives would be appreciated.

Thanks,
Richard

[PATCH 1/2] tap: new `--merge-prefix' option to prefix stderr before merging

* lib/tap-driver.sh: Add a new `--merge-prefix' option to direct the TAP
driver to prefix each line of the test script's standard error with the given
string before merging it with the test script's standard output.  This is
useful when standard error lines might be confused with test results.
* doc/automake.texi: Document the new `--merge-prefix' option.
* t/tap-merge-prefix.sh: New test.
* t/list-of-tests.mk (handwritten_TESTS): Add it.

[PATCH 2/2] tap: close fd 3 when invoking the test script

* lib/tap-driver.sh: Close file descriptor 3 when invoking the test script to
avoid potential conflicts with the test script.
From a8cbb5a235f89e127aef836a12e5813de112cb91 Mon Sep 17 00:00:00 2001
From: Richard Hansen <[email-masked]>
Date: Thu, 8 Aug 2024 23:34:45 -0400
Subject: [PATCH 1/2] tap: new `--merge-prefix' option to prefix stderr before
 merging

* lib/tap-driver.sh: Add a new `--merge-prefix' option to direct the TAP
driver to prefix each line of the test script's standard error with the given
string before merging it with the test script's standard output.  This is
useful when standard error lines might be confused with test results.
* doc/automake.texi: Document the new `--merge-prefix' option.
* t/tap-merge-prefix.sh: New test.
* t/list-of-tests.mk (handwritten_TESTS): Add it.
---
 doc/automake.texi     |  9 ++++++-
 lib/tap-driver.sh     | 20 ++++++++++++----
 t/list-of-tests.mk    |  1 +
 t/tap-merge-prefix.sh | 56 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 6 deletions(-)
 create mode 100644 t/tap-merge-prefix.sh

diff --git a/doc/automake.texi b/doc/automake.texi
index 49ebfb2b2..a1b02642c 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -10488,9 +10488,16 @@ relative to test results; this can be of great help in debugging
 (especially if your test scripts are shell scripts run with shell
 tracing active).  As a downside, this option might cause the test
 harness to get confused if anything that appears on standard error
-looks like a test result.
+looks like a test result.  Use @option{--merge-prefix} to prefix each
+line of standard error to avoid this problem.
 @item --no-merge
 Revert the effects of @option{--merge}.
+@item --merge-prefix @var{STRING}
+If @option{--merge} is in effect, prefix each line of the test script's
+standard error with @code{@var{STRING}} before merging it into standard
+output.  Typically @code{@var{STRING}} should begin with ``@code{#}'',
+which starts a TAP comment.  Defaults to the empty string.  This option
+has no effect if @option{--merge} is not in effect.
 @item --diagnostic-string @var{STRING}
 Change the string that introduces TAP diagnostics from the default value
 of ``@code{#}'' to @code{@var{STRING}}.  This can be useful if your
diff --git a/lib/tap-driver.sh b/lib/tap-driver.sh
index bd9597588..ef5057fc5 100755
--- a/lib/tap-driver.sh
+++ b/lib/tap-driver.sh
@@ -52,7 +52,8 @@ Usage:
                 [--expect-failure {yes|no}] [--color-tests {yes|no}]
                 [--enable-hard-errors {yes|no}] [--ignore-exit]
                 [--diagnostic-string STRING] [--merge|--no-merge]
-                [--comments|--no-comments] [--] TEST-COMMAND
+                [--merge-prefix STRING] [--comments|--no-comments]
+                [--] TEST-COMMAND
 The '--test-name', '-log-file' and '--trs-file' options are mandatory.
 
 Report bugs to <[email-masked]>.
@@ -69,6 +70,7 @@ trs_file=  # Where to save the metadata of the test run.
 expect_failure=0
 color_tests=0
 merge=0
+merge_prefix=
 ignore_exit=0
 comments=0
 diag_string='#'
@@ -84,6 +86,7 @@ while test $# -gt 0; do
   --enable-hard-errors) shift;; # No-op.
   --merge) merge=1;;
   --no-merge) merge=0;;
+  --merge-prefix) merge_prefix=$2; shift;;
   --ignore-exit) ignore_exit=1;;
   --comments) comments=1;;
   --no-comments) comments=0;;
@@ -140,12 +143,19 @@ fi
     # <http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2009-February/004121.html>
     trap : 1 3 2 13 15
     if test $merge -gt 0; then
-      exec 2>&1
+      exec 4>&1
+      # A shell `while' loop is used to read and prefix the stderr
+      # lines instead of `awk' because `mawk' aggressively buffers its
+      # input (except with the `-Winteractive' command-line option),
+      # which defeats the purpose of merging the output.
+      { "$@"; echo $?; } 2>&1 >&4 4>&- | while IFS= read -r line; do
+        printf %s\\n "$merge_prefix$line"
+      done
+      exec 4>&-
     else
-      exec 2>&3
+      "$@" 2>&3
+      echo $?
     fi
-    "$@"
-    echo $?
   ) | LC_ALL=C ${AM_TAP_AWK-awk} \
         -v me="$me" \
         -v test_script_name="$test_name" \
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index e80ace470..ad8976738 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1153,6 +1153,7 @@ t/tap-msg0-directive.sh \
 t/tap-msg0-planskip.sh \
 t/tap-msg0-bailout.sh \
 t/tap-msg0-misc.sh \
+t/tap-merge-prefix.sh \
 t/tap-merge-stdout-stderr.sh \
 t/tap-no-merge-stdout-stderr.sh \
 t/tap-no-disable-hard-error.sh \
diff --git a/t/tap-merge-prefix.sh b/t/tap-merge-prefix.sh
new file mode 100644
index 000000000..0ed0929b6
--- /dev/null
+++ b/t/tap-merge-prefix.sh
@@ -0,0 +1,56 @@
+#! /bin/sh
+# Copyright (C) 2024 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 <https://www.gnu.org/licenses/>.
+
+# TAP support:
+#  - The Automake TAP driver has an option that instructs it to prefix stderr
+#    with a given string before merging it with stdout.
+
+. test-init.sh
+
+fetch_tap_driver
+
+# There's apparently no portable way to escape # in a make variable, so use
+# printf with an octal escape for the character's representation.  We could
+# just $(printf '\043'), but that doesn't work on EBCDIC systems so the octal
+# representation is computed here.
+octothorpe_octal=$(printf %03o "'#")
+cat > Makefile.am <<END
+AM_TEST_LOG_DRIVER_FLAGS = --merge \\
+	--merge-prefix "\$\$(printf '\\$octothorpe_octal')<stderr> "
+TESTS = all.test
+END
+
+cat Makefile.am  # for debugging
+
+. tap-setup.sh
+
+cat > all.test <<END
+#!/bin/sh
+echo 1..2
+echo ok 1
+echo 'Bail out!' >&2
+echo ok 2
+END
+chmod a+x all.test
+
+run_make -O check
+count_test_results total=2 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=0
+
+cat all.log  # for debugging
+
+grep -e '^#<stderr> Bail out!$' all.log
+
+:
-- 
2.40.1

From 7414e7fc942512e4c8084fbee7515488b3a3e2e5 Mon Sep 17 00:00:00 2001
From: Richard Hansen <[email-masked]>
Date: Thu, 8 Aug 2024 23:41:40 -0400
Subject: [PATCH 2/2] tap: close fd 3 when invoking the test script

* lib/tap-driver.sh: Close file descriptor 3 when invoking the test script to
avoid potential conflicts with the test script.
---
 lib/tap-driver.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/tap-driver.sh b/lib/tap-driver.sh
index ef5057fc5..8a59b423b 100755
--- a/lib/tap-driver.sh
+++ b/lib/tap-driver.sh
@@ -148,12 +148,12 @@ fi
       # lines instead of `awk' because `mawk' aggressively buffers its
       # input (except with the `-Winteractive' command-line option),
       # which defeats the purpose of merging the output.
-      { "$@"; echo $?; } 2>&1 >&4 4>&- | while IFS= read -r line; do
+      { "$@"; echo $?; } 2>&1 >&4 3>&- 4>&- | while IFS= read -r line; do
         printf %s\\n "$merge_prefix$line"
       done
       exec 4>&-
     else
-      "$@" 2>&3
+      "$@" 2>&3 3>&-
       echo $?
     fi
   ) | LC_ALL=C ${AM_TAP_AWK-awk} \
@@ -648,8 +648,6 @@ exit 0
 
 } # End of "BEGIN" block.
 '
-
-# TODO: document that we consume the file descriptor 3 :-(
 } 3>"$log_file"
 
 test $? -eq 0 || fatal "I/O or internal error"
-- 
2.40.1