tests: fix --write-junit-xml with subshells

From: Johannes Schindelin <[email protected]>

In t0000, more precisely in its test_bool_env test case, there are two subshells that are supposed to fail. To be even _more_ precise, they fail by calling the error function, and that is okay, because it is in a subshell, and it is expected that those two subshell invocations fail.

However, the error function also tries to finalize the JUnit XML (if that XML was asked for, via --write-junit-xml. As a consequence, the XML is edited to add a time attribute for the testsuite tag. And since there are two expected error calls in addition to the final test_done, the finalize_junit_xml function is called three times and naturally the time attribute is added _three times_.

Azure Pipelines is not happy with that, complaining thusly:

##[warning]Failed to read D:\a\1\s\t\out\TEST-t0000-basic.xml. Error : 'time' is a duplicate attribute name. Line 2, position 82..

One possible way to address this would be to unset write_junit_xml in the test_bool_env test case.

But that would be fragile, as other error calls in subshells could be introduced.

So let's just modify finalize_junit_xml to remove any time attribute before adding the authoritative one.

---
Fix a warning in the Azure Pipelines runs

Yet another thing I stumbled across...

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-552%2Fdscho%2Fwrite-junit-xml-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-552/dscho/write-junit-xml-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/552

 t/test-lib.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 44df51be8f..0ea1e5a05e 100644
@@ -1083,7 +1083,8 @@ finalize_junit_xml () {
 
 		# adjust the overall time
 		junit_time=$(test-tool date getnanos $junit_suite_start)
-		sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
+		sed -e "s/\(<testsuite.*\) time=\"[^\"]*\"/\1/" \
+			-e "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
 			<"$junit_xml_path" >"$junit_xml_path.new"
 		mv "$junit_xml_path.new" "$junit_xml_path"
 

base-commit: de93cc14ab7e8db7645d8dbe4fd2603f76d5851f

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Interesting. Naïvely, I would have expected that the right fix, especially after reading "as other error calls can be introduced" was to stop calling finalize from error() and instead call it from the exit trap, but that would not work, as the exit trap would also be called even from a subshell X-<.

It feels wrong to repeatedly rewrite the $junit_xml_path file during the test, and it feels doubly wrong to call finalize many number of times, but given that we do not have a good way to revamp the code to stop doing so, I think "strip existing time attribute, if any, and then add what we have" we see here is the best solution available.

Will queue. Thanks.