Re: [PATCH 6/7] test-lib: make --verbose output valid TAP
From: Chris Torek <hidden>
Date: 2021-03-10 07:43:40
There is a lot of historical stuff I can't really comment on here, so I'll just point out this one tiny bit of shell oddity: On Tue, Mar 9, 2021 at 8:04 AM Ævar Arnfjörð Bjarmason [off-list ref] wrote:
Make the --verbose output be valid TAP, making it machine-readable for TAP parsers again.
[snippage]
quoted hunk ↗ jump to hunk
diff --git a/t/test-lib.sh b/t/test-lib.sh index aa7068b06b6..0070d05234b 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh
[snippage]
quoted hunk ↗ jump to hunk
@@ -364,9 +366,22 @@ then ( GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1 echo $? >"$TEST_RESULTS_BASE.exit" - ) | tee -a "$GIT_TEST_TEE_OUTPUT_FILE" + ) | "$GIT_BUILD_DIR"/t/helper/test-tool tee \ + --tap --prefix="GIT_TEST_TEE_STARTED " \ + --escape-stdout ${HARNESS_ACTIVE+--escape-file} \ + "$GIT_TEST_TEE_OUTPUT_FILE" test "$(cat "$TEST_RESULTS_BASE.exit")" = 0 exit +elif test -n "$verbose" -a -n "$HARNESS_ACTIVE" +then + ret= + ( + GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1 + ret=$? + ) | "$GIT_BUILD_DIR"/t/helper/test-tool tee \ + --tap --prefix="GIT_TEST_TEE_STARTED " \ + --escape-stdout + exit $ret fi if test -n "$trace" && test -n "$test_untraceable"
[snippage]
In the block beginning with:
ret=
we have a subshell:
( ... ) | "$GIT_BUILD_DIR"/t/helper/test-tool tee ...
In the subshell itself, we set `ret=$?`. But this is inside a
subshell, which then exits, so the setting of `ret` will get lost.
Did you perhaps want `{ ...; }` instead here? Unfortunately the
pipe means that the whole left side may run in a subshell
anyway, so even that doesn't fix the problem. We need a
temp file, a la the code above that dumps $? into
"$TEST_RESULTS_BASE.exit".
Chris