Subject: [bug#72536] [PATCH v2] tap: new `--stderr-prefix' option to prefix test script's, stderr

From: Richard Hansen <[email-masked]>
Date: Mon, 12 Aug 2024 03:58:48 -0400

Attached is v2 of the patch series.  Changes from v1:

   * Renamed the option from `--merge-prefix' to `--stderr-prefix'
   * stderr is now always prefixed (even with `--no-merge')
   * Added Autoconf-style quadrigraphs @%:@ (for #) and @&t@ (for empty 
string)
   * Bumped `scriptversion'
   * Reordered the patches (close fd 3 is the 1st patch now)
   * Added examples to the documentation
   * Reworked the tests:
      - `#' is now passed via command-line variable assignment instead 
of in the Makefile, avoiding the octal printf mess
      - quadrigraph replacement is tested
      - tested with and without --merge

I expect to hear back from [email-masked] today (Monday) with the 
copyright assignment agreement; I'll get that signed and sent ASAP.

Thanks,
Richard
From 4b914acebc1e5e691bd88fc01549208303cd2609 Mon Sep 17 00:00:00 2001
From: Richard Hansen <[email-masked]>
Date: Mon, 12 Aug 2024 02:03:31 -0400
Subject: [PATCH 1/2] tap: close fd 3 before invoking the test script

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

diff --git a/lib/tap-driver.sh b/lib/tap-driver.sh
index bd9597588..39c3a035e 100755
--- a/lib/tap-driver.sh
+++ b/lib/tap-driver.sh
@@ -23,7 +23,7 @@
 # bugs to <[email-masked]> or send patches to
 # <[email-masked]>.
 
-scriptversion=2024-06-19.01; # UTC
+scriptversion=2024-08-12.01; # UTC
 
 # Make unconditional expansion of undefined variables an error.  This
 # helps a lot in preventing typo-related bugs.
@@ -144,6 +144,7 @@ fi
     else
       exec 2>&3
     fi
+    exec 3>&-
     "$@"
     echo $?
   ) | LC_ALL=C ${AM_TAP_AWK-awk} \
@@ -638,8 +639,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

From 766fc48ae358fc73a1eb81758a239b47373419e3 Mon Sep 17 00:00:00 2001
From: Richard Hansen <[email-masked]>
Date: Mon, 12 Aug 2024 02:03:58 -0400
Subject: [PATCH 2/2] tap: new `--stderr-prefix' option to prefix test script's
 stderr

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

diff --git a/doc/automake.texi b/doc/automake.texi
index 49ebfb2b2..7e770f74a 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -10488,9 +10488,35 @@ 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{--stderr-prefix} to prefix each
+line of standard error to avoid this problem.
 @item --no-merge
 Revert the effects of @option{--merge}.
+@item --stderr-prefix @var{STRING}
+Prefix each line of the test script's standard error with
+@code{@var{STRING}}.  Defaults to the empty string.  This option makes
+it possible to avoid parsing ambiguity when @option{--merge} is in use,
+but it can also be used without @option{--merge} to make it easier to
+read test log files.  When used with @option{--merge}, users will
+typically want to start @code{@var{STRING}} with @samp{#}, which starts
+a TAP comment.  The @samp{#} character is difficult to portably include
+in a @command{make} variable, so the following quadrigraph
+substitutions, copied from Autoconf (@pxref{Quadrigraphs, , , autoconf,
+The Autoconf Manual}), are performed on @code{@var{STRING}}:
+
+@table @samp
+@item @@%:@@
+Replaced with @samp{#}.
+@item @@&t@@
+Replaced with the empty string.  This makes it possible to include the
+literal characters @samp{@@%:@@} in the prefix (@samp{@@%@@&t@@:@@}, for
+example).
+@end table
+
+For example, in shell, @samp{--stderr-prefix "@@%:@@<stderr> "} will
+prefix each line of the test script's standard error with
+@samp{#<stderr> }.
+
 @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
@@ -10578,6 +10604,58 @@ PASS: baz.test 1
 ...
 % @kbd{echo exit status: $?}
 exit status: 0
+
+% @kbd{cat stderr-demo.test}
+#!/bin/sh
+echo 'ok 1'
+# These sleeps help highlight the reordering of standard output and
+# standard error lines that can happen if --merge is not used.
+sleep 1
+# Note that this is a diagnostic message written to standard error, so
+# it should not affect the tests.  However, it will break the tests if
+# --merge is used without --stderr-prefix because "Bail out!" will be
+# interpreted as a TAP directive.
+echo 'Bail out! (this should not actually bail out)' >&2
+sleep 1
+echo 'ok 2'
+echo '1..2'
+
+% @kbd{make check TESTS=stderr-demo.test}
+...
+PASS: stderr-demo.test 1
+PASS: stderr-demo.test 2
+...
+
+% @kbd{cat stderr-demo.log}
+Bail out! (this should not actually bail out)
+ok 1
+PASS: stderr-demo.test 1
+ok 2
+PASS: stderr-demo.test 2
+1..2
+
+% @kbd{make check TESTS=stderr-demo.test \
+      TEST_LOG_DRIVER_FLAGS='--merge'}
+...
+PASS: stderr-demo.test 1
+ERROR: stderr-demo.test - Bail out! (this should not actually bail out)
+...
+
+% @kbd{make check TESTS=stderr-demo.test \
+      TEST_LOG_DRIVER_FLAGS='--merge --stderr-prefix "@@%:@@<stderr> "'}
+...
+PASS: stderr-demo.test 1
+PASS: stderr-demo.test 2
+...
+
+% @kbd{cat stderr-demo.log}
+ok 1
+PASS: stderr-demo.test 1
+#<stderr> Bail out! (this should not actually bail out)
+ok 2
+PASS: stderr-demo.test 2
+1..2
+
 @end example
 
 @node Incompatibility with other TAP parsers and drivers
diff --git a/lib/tap-driver.sh b/lib/tap-driver.sh
index 39c3a035e..71ad1d8bd 100755
--- a/lib/tap-driver.sh
+++ b/lib/tap-driver.sh
@@ -23,7 +23,7 @@
 # bugs to <[email-masked]> or send patches to
 # <[email-masked]>.
 
-scriptversion=2024-08-12.01; # UTC
+scriptversion=2024-08-12.02; # UTC
 
 # Make unconditional expansion of undefined variables an error.  This
 # helps a lot in preventing typo-related bugs.
@@ -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
+                [--stderr-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
+stderr_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;;
+  --stderr-prefix) stderr_prefix=$2; shift;;
   --ignore-exit) ignore_exit=1;;
   --comments) comments=1;;
   --no-comments) comments=0;;
@@ -94,6 +97,20 @@ while test $# -gt 0; do
   shift
 done
 
+# Quadrigraph substitutions for `--stderr-prefix'.  Note that the
+# empty substitution MUST be done last, otherwise `@%@&t@:@' will
+# become `#', not `@%:@'.
+for q_r in '@%:@ #' '@&t@ '; do
+  q=${q_r%% *} # quadrigraph
+  r=${q_r#* } # replacement
+  while true; do
+    case $stderr_prefix in
+    *"$q"*) stderr_prefix=${stderr_prefix%%"$q"*}$r${stderr_prefix#*"$q"};;
+    *) break;;
+    esac
+  done
+done
+
 test $# -gt 0 || usage_error "missing test command"
 
 case $expect_failure in
@@ -144,9 +161,17 @@ fi
     else
       exec 2>&3
     fi
+    # FD 3 is not needed in this subshell anymore, so reuse it for prefixing
+    # stderr.
+    exec 3>&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
+    # would defeat the purpose of the `--merge' option.
+    { "$@"; echo $?; } 2>&1 >&3 3>&- | while IFS= read -r line; do
+      printf %s\\n "$stderr_prefix$line" >&2
+    done
     exec 3>&-
-    "$@"
-    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..b29d4d96d 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1185,6 +1185,7 @@ t/tap-planskip-whitespace.sh \
 t/tap-planskip-badexit.sh \
 t/tap-planskip-bailout.sh \
 t/tap-planskip-later-errors.sh \
+t/tap-stderr-prefix.sh \
 t/tap-test-number-0.sh \
 t/tap-recheck-logs.sh \
 t/tap-result-comment.sh \
diff --git a/t/tap-stderr-prefix.sh b/t/tap-stderr-prefix.sh
new file mode 100644
index 000000000..7659afd66
--- /dev/null
+++ b/t/tap-stderr-prefix.sh
@@ -0,0 +1,76 @@
+#! /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 the
+#    test script's stderr with a given string.
+
+. test-init.sh
+
+fetch_tap_driver
+
+cat > Makefile.am <<END
+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
+
+# Baseline tests without --stderr-prefix.
+
+run_make -O AM_TEST_LOG_DRIVER_FLAGS= check
+count_test_results total=2 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=0
+cat all.log  # for debugging
+grep '^Bail out!$' all.log
+
+run_make -e FAIL AM_TEST_LOG_DRIVER_FLAGS=--merge check
+# Don't bother checking the counts -- they're meaningless due to the merged
+# stderr "corrupting" the TAP output.  It is sufficient to just test that
+# `make check' returned non-zero.
+cat all.log  # for debugging
+grep '^Bail out!$' all.log
+
+# Now add --stderr-prefix.  Included in the prefix:
+#   1. quadrigraph for # (@%:@)
+#   2. literal # (difficult to include in a Makefile variable, but easy to
+#      pass in a command-line make variable assignment)
+#   3. a string that expands to @%:@ (quadrigraph for #) after quadrigraph
+#      replacement, accomplished by embedding a quadrigraph for the empty
+#      string inside the quadrigraph for #
+# The end result should be: ##@%:@
+PFXFLAG='--stderr-prefix "@%:@#@%@&t@:@ "'
+
+run_make -O AM_TEST_LOG_DRIVER_FLAGS="$PFXFLAG" check
+count_test_results total=2 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=0
+cat all.log  # for debugging
+grep '^##@%:@ Bail out!$' all.log
+
+run_make -O AM_TEST_LOG_DRIVER_FLAGS="--merge $PFXFLAG" check
+cat all.log  # for debugging
+count_test_results total=2 pass=2 fail=0 xpass=0 xfail=0 skip=0 error=0
+grep '^##@%:@ Bail out!$' all.log
+
+:
-- 
2.40.1