git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 02/12] tests: refactor --write-junit-xml code

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
May 21, 2022, 22:18 UTC
Message-ID
<cbf476e4e988cd24ed42fd6f5ddaa7840a33583b.1653171536.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1117.v3.git.1653171536.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The code writing JUnit XML is interspersed directly with all the code in `t/test-lib.sh`, and it is therefore not only ill-separated, but introducing yet another output format would make the situation even worse.

Let's introduce an abstraction layer by hiding the JUnit XML code behind four new functions that are supposed to be called before and after each test and test case.

This is not just an academic exercise, refactoring for refactoring's sake. We _actually_ want to introduce such a new output format, to make it substantially easier to diagnose test failures in our GitHub workflow, therefore we do need this refactoring.

This commit is best viewed with `git show --color-moved --color-moved-ws=allow-indentation-change <commit>`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/test-lib-junit.sh | 126 ++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh       | 124 ++++++-------------------------------------
 2 files changed, 142 insertions(+), 108 deletions(-)
 create mode 100644 t/test-lib-junit.sh
diff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh
new file mode 100644
index 00000000000..9d55d74d764
--- /dev/null
+++ b/t/test-lib-junit.sh
@@ -0,0 +1,126 @@
+# Library of functions to format test scripts' output in JUnit XML
+# format, to support Git's test suite result to be presented in an
+# easily digestible way on Azure Pipelines.
+#
+# Copyright (c) 2022 Johannes Schindelin
+#
+# 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 of the License, 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 http://www.gnu.org/licenses/ .
+#
+# The idea is for `test-lib.sh` to source this file when the user asks
+# for JUnit XML; these functions will then override (empty) functions
+# that are are called at the appropriate times during the test runs.
+
+start_test_output () {
+	junit_xml_dir="$TEST_OUTPUT_DIRECTORY/out"
+	mkdir -p "$junit_xml_dir"
+	junit_xml_base=${1##*/}
+	junit_xml_path="$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml"
+	junit_attrs="name=\"${junit_xml_base%.sh}\""
+	junit_attrs="$junit_attrs timestamp=\"$(TZ=UTC \
+		date +%Y-%m-%dT%H:%M:%S)\""
+	write_junit_xml --truncate "<testsuites>" "  <testsuite $junit_attrs>"
+	junit_suite_start=$(test-tool date getnanos)
+	if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
+	then
+		GIT_TEST_TEE_OFFSET=0
+	fi
+}
+
+start_test_case_output () {
+	junit_start=$(test-tool date getnanos)
+}
+
+finalize_test_case_output () {
+	test_case_result=$1
+	shift
+	case "$test_case_result" in
+	ok)
+		set "$*"
+		;;
+	failure)
+		junit_insert="<failure message=\"not ok $test_count -"
+		junit_insert="$junit_insert $(xml_attr_encode "$1")\">"
+		junit_insert="$junit_insert $(xml_attr_encode \
+			"$(if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
+			   then
+				test-tool path-utils skip-n-bytes \
+					"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
+			   else
+				printf '%s\n' "$@" | sed 1d
+			   fi)")"
+		junit_insert="$junit_insert</failure>"
+		if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
+		then
+			junit_insert="$junit_insert<system-err>$(xml_attr_encode \
+				"$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")</system-err>"
+		fi
+		set "$1" "      $junit_insert"
+		;;
+	fixed)
+		set "$* (breakage fixed)"
+		;;
+	broken)
+		set "$* (known breakage)"
+		;;
+	skip)
+		message="$(xml_attr_encode "$skipped_reason")"
+		set "$1" "      <skipped message=\"$message\" />"
+		;;
+	esac
+
+	junit_attrs="name=\"$(xml_attr_encode "$this_test.$test_count $1")\""
+	shift
+	junit_attrs="$junit_attrs classname=\"$this_test\""
+	junit_attrs="$junit_attrs time=\"$(test-tool \
+		date getnanos $junit_start)\""
+	write_junit_xml "$(printf '%s\n' \
+		"    <testcase $junit_attrs>" "$@" "    </testcase>")"
+	junit_have_testcase=t
+}
+
+finalize_test_output () {
+	if test -n "$junit_xml_path"
+	then
+		test -n "$junit_have_testcase" || {
+			junit_start=$(test-tool date getnanos)
+			write_junit_xml_testcase "all tests skipped"
+		}
+
+		# adjust the overall time
+		junit_time=$(test-tool date getnanos $junit_suite_start)
+		sed -e "s/\(<testsuite.*\) time=\"[^\"]*\"/\1/" \
+			-e "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
+			-e '/^ *<\/testsuite/d' \
+			<"$junit_xml_path" >"$junit_xml_path.new"
+		mv "$junit_xml_path.new" "$junit_xml_path"
+
+		write_junit_xml "  </testsuite>" "</testsuites>"
+		write_junit_xml=
+	fi
+}
+
+write_junit_xml () {
+	case "$1" in
+	--truncate)
+		>"$junit_xml_path"
+		junit_have_testcase=
+		shift
+		;;
+	esac
+	printf '%s\n' "$@" >>"$junit_xml_path"
+}
+
+xml_attr_encode () {
+	printf '%s\n' "$@" | test-tool xml-encode
+}
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f09e8f3efce..bdb11e28eea 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -137,6 +137,12 @@ mark_option_requires_arg () {
 	store_arg_to=$2
 }
 
+# These functions can be overridden e.g. to output JUnit XML
+start_test_output () { :; }
+start_test_case_output () { :; }
+finalize_test_case_output () { :; }
+finalize_test_output () { :; }
+
 parse_option () {
 	local opt="$1"
 
@@ -196,7 +202,7 @@ parse_option () {
 		tee=t
 		;;
 	--write-junit-xml)
-		write_junit_xml=t
+		. "$TEST_DIRECTORY/test-lib-junit.sh"
 		;;
 	--stress)
 		stress=t ;;
@@ -664,7 +670,7 @@ exec 6<&0
 exec 7>&2
 
 _error_exit () {
-	finalize_junit_xml
+	finalize_test_output
 	GIT_EXIT_OK=t
 	exit 1
 }
@@ -774,35 +780,13 @@ trap '{ code=$?; set +x; } 2>/dev/null; exit $code' INT TERM HUP
 # the test_expect_* functions instead.
 
 test_ok_ () {
-	if test -n "$write_junit_xml"
-	then
-		write_junit_xml_testcase "$*"
-	fi
+	finalize_test_case_output ok "$@"
 	test_success=$(($test_success + 1))
 	say_color "" "ok $test_count - $@"
 }
 
 test_failure_ () {
-	if test -n "$write_junit_xml"
-	then
-		junit_insert="<failure message=\"not ok $test_count -"
-		junit_insert="$junit_insert $(xml_attr_encode "$1")\">"
-		junit_insert="$junit_insert $(xml_attr_encode \
-			"$(if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
-			   then
-				test-tool path-utils skip-n-bytes \
-					"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
-			   else
-				printf '%s\n' "$@" | sed 1d
-			   fi)")"
-		junit_insert="$junit_insert</failure>"
-		if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
-		then
-			junit_insert="$junit_insert<system-err>$(xml_attr_encode \
-				"$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")</system-err>"
-		fi
-		write_junit_xml_testcase "$1" "      $junit_insert"
-	fi
+	finalize_test_case_output failure "$@"
 	test_failure=$(($test_failure + 1))
 	say_color error "not ok $test_count - $1"
 	shift
@@ -815,19 +799,13 @@ test_failure_ () {
 }
 
 test_known_broken_ok_ () {
-	if test -n "$write_junit_xml"
-	then
-		write_junit_xml_testcase "$* (breakage fixed)"
-	fi
+	finalize_test_case_output fixed "$@"
 	test_fixed=$(($test_fixed+1))
 	say_color error "ok $test_count - $@ # TODO known breakage vanished"
 }
 
 test_known_broken_failure_ () {
-	if test -n "$write_junit_xml"
-	then
-		write_junit_xml_testcase "$* (known breakage)"
-	fi
+	finalize_test_case_output broken "$@"
 	test_broken=$(($test_broken+1))
 	say_color warn "not ok $test_count - $@ # TODO known breakage"
 }
@@ -1104,10 +1082,7 @@ test_start_ () {
 	test_count=$(($test_count+1))
 	maybe_setup_verbose
 	maybe_setup_valgrind
-	if test -n "$write_junit_xml"
-	then
-		junit_start=$(test-tool date getnanos)
-	fi
+	start_test_case_output
 }
 
 test_finish_ () {
@@ -1158,12 +1133,7 @@ test_skip () {
 
 	case "$to_skip" in
 	t)
-		if test -n "$write_junit_xml"
-		then
-			message="$(xml_attr_encode "$skipped_reason")"
-			write_junit_xml_testcase "$1" \
-				"      <skipped message=\"$message\" />"
-		fi
+		finalize_test_case_output skip "$@"
 
 		say_color skip "ok $test_count # skip $1 ($skipped_reason)"
 		: true
@@ -1179,53 +1149,6 @@ test_at_end_hook_ () {
 	:
 }
 
-write_junit_xml () {
-	case "$1" in
-	--truncate)
-		>"$junit_xml_path"
-		junit_have_testcase=
-		shift
-		;;
-	esac
-	printf '%s\n' "$@" >>"$junit_xml_path"
-}
-
-xml_attr_encode () {
-	printf '%s\n' "$@" | test-tool xml-encode
-}
-
-write_junit_xml_testcase () {
-	junit_attrs="name=\"$(xml_attr_encode "$this_test.$test_count $1")\""
-	shift
-	junit_attrs="$junit_attrs classname=\"$this_test\""
-	junit_attrs="$junit_attrs time=\"$(test-tool \
-		date getnanos $junit_start)\""
-	write_junit_xml "$(printf '%s\n' \
-		"    <testcase $junit_attrs>" "$@" "    </testcase>")"
-	junit_have_testcase=t
-}
-
-finalize_junit_xml () {
-	if test -n "$write_junit_xml" && test -n "$junit_xml_path"
-	then
-		test -n "$junit_have_testcase" || {
-			junit_start=$(test-tool date getnanos)
-			write_junit_xml_testcase "all tests skipped"
-		}
-
-		# adjust the overall time
-		junit_time=$(test-tool date getnanos $junit_suite_start)
-		sed -e "s/\(<testsuite.*\) time=\"[^\"]*\"/\1/" \
-			-e "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
-			-e '/^ *<\/testsuite/d' \
-			<"$junit_xml_path" >"$junit_xml_path.new"
-		mv "$junit_xml_path.new" "$junit_xml_path"
-
-		write_junit_xml "  </testsuite>" "</testsuites>"
-		write_junit_xml=
-	fi
-}
-
 test_atexit_cleanup=:
 test_atexit_handler () {
 	# In a succeeding test script 'test_atexit_handler' is invoked
@@ -1248,7 +1171,7 @@ test_done () {
 	# removed, so the commands can access pidfiles and socket files.
 	test_atexit_handler
 
-	finalize_junit_xml
+	finalize_test_output
 
 	if test -z "$HARNESS_ACTIVE"
 	then
@@ -1539,22 +1462,7 @@ fi
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$TRASH_DIRECTORY" || exit 1
 
-if test -n "$write_junit_xml"
-then
-	junit_xml_dir="$TEST_OUTPUT_DIRECTORY/out"
-	mkdir -p "$junit_xml_dir"
-	junit_xml_base=${0##*/}
-	junit_xml_path="$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml"
-	junit_attrs="name=\"${junit_xml_base%.sh}\""
-	junit_attrs="$junit_attrs timestamp=\"$(TZ=UTC \
-		date +%Y-%m-%dT%H:%M:%S)\""
-	write_junit_xml --truncate "<testsuites>" "  <testsuite $junit_attrs>"
-	junit_suite_start=$(test-tool date getnanos)
-	if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
-	then
-		GIT_TEST_TEE_OFFSET=0
-	fi
-fi
+start_test_output "$0"
 
 # Convenience
 # A regexp to match 5 and 35 hexdigits

-- 
gitgitgadget
Previous: Johannes Schindelin via GitGitGadgetNext: Johannes Schindelin via GitGitGadget
Message 87 of 98 in “ci: make Git's GitHub workflow output much more helpful”
  1. 0/9 ci: make Git's GitHub workflow output much more helpfulJohannes Schindelin via GitGitGadget, Jan 24, 2022
  2. 1/9 ci: fix code styleJohannes Schindelin via GitGitGadget, Jan 24, 2022
  3. 2/9 ci/run-build-and-tests: take a more high-level viewJohannes Schindelin via GitGitGadget, Jan 24, 2022
  4. Eric SunshineJan 24, 2022
  5. Johannes SchindelinJan 25, 2022
  6. 3/9 ci: make it easier to find failed tests' logs in the GitHub workflowJohannes Schindelin via GitGitGadget, Jan 24, 2022
  7. Ævar Arnfjörð BjarmasonJan 25, 2022
  8. 4/9 ci/run-build-and-tests: add some structure to the GitHub workflow outputJohannes Schindelin via GitGitGadget, Jan 24, 2022
  9. Phillip WoodFeb 23, 2022
  10. Johannes SchindelinFeb 25, 2022
  11. 5/9 tests: refactor --write-junit-xml codeJohannes Schindelin via GitGitGadget, Jan 24, 2022
  12. Ævar Arnfjörð BjarmasonJan 26, 2022
  13. 6/9 test(junit): avoid line feeds in XML attributesJohannes Schindelin via GitGitGadget, Jan 24, 2022
  14. 9/9 ci: call `finalize_test_case_output` a little laterJohannes Schindelin via GitGitGadget, Jan 24, 2022
  15. 8/9 ci: use `--github-workflow-markup` in the GitHub workflowJohannes Schindelin via GitGitGadget, Jan 24, 2022
  16. 7/9 ci: optionally mark up output in the GitHub workflowJohannes Schindelin via GitGitGadget, Jan 24, 2022
  17. Ævar Arnfjörð BjarmasonJan 26, 2022
  18. CI "grouping" within jobs v.s. lighter split-out jobs (was: [PATCH 0/9] ci: make Git's GitHub workflow output much more helpful)Ævar Arnfjörð Bjarmason, Jan 27, 2022
  19. Johannes SchindelinFeb 19, 2022
  20. Junio C HamanoFeb 20, 2022
  21. Johannes SchindelinFeb 20, 2022
  22. Ævar Arnfjörð BjarmasonFeb 21, 2022
  23. Johannes SchindelinFeb 22, 2022
  24. Ævar Arnfjörð BjarmasonFeb 20, 2022
  25. Johannes SchindelinFeb 22, 2022
  26. Ævar Arnfjörð BjarmasonFeb 22, 2022
  27. Phillip WoodFeb 23, 2022
  28. Ævar Arnfjörð BjarmasonFeb 25, 2022
  29. Johannes SchindelinFeb 25, 2022
  30. Junio C HamanoFeb 25, 2022
  31. Junio C HamanoFeb 26, 2022
  32. Junio C HamanoMar 1, 2022
  33. Junio C HamanoMar 1, 2022
  34. Johannes SchindelinMar 1, 2022
  35. Junio C HamanoMar 1, 2022
  36. Johannes SchindelinMar 1, 2022
  37. Junio C HamanoMar 1, 2022
  38. Johannes SchindelinMar 1, 2022
  39. win+VS environment has "cut" but not "paste"?Junio C Hamano, Mar 4, 2022
  40. Ævar Arnfjörð BjarmasonMar 4, 2022
  41. Johannes SchindelinMar 7, 2022
  42. Junio C HamanoMar 7, 2022
  43. Johannes SchindelinMar 9, 2022
  44. Ævar Arnfjörð BjarmasonMar 10, 2022
  45. Johannes SchindelinMar 7, 2022
  46. Junio C HamanoMar 7, 2022
  47. Phillip WoodMar 2, 2022
  48. Johannes SchindelinMar 7, 2022
  49. Junio C HamanoMar 7, 2022
  50. Ævar Arnfjörð BjarmasonMar 9, 2022
  51. Phillip WoodMar 7, 2022
  52. 0/9 ci: make Git's GitHub workflow output much more helpfulJohannes Schindelin via GitGitGadget, Mar 1, 2022
  53. 1/9 ci: fix code styleJohannes Schindelin via GitGitGadget, Mar 1, 2022
  54. 2/9 ci/run-build-and-tests: take a more high-level viewJohannes Schindelin via GitGitGadget, Mar 1, 2022
  55. 3/9 ci: make it easier to find failed tests' logs in the GitHub workflowJohannes Schindelin via GitGitGadget, Mar 1, 2022
  56. 4/9 ci/run-build-and-tests: add some structure to the GitHub workflow outputJohannes Schindelin via GitGitGadget, Mar 1, 2022
  57. 5/9 tests: refactor --write-junit-xml codeJohannes Schindelin via GitGitGadget, Mar 1, 2022
  58. 6/9 test(junit): avoid line feeds in XML attributesJohannes Schindelin via GitGitGadget, Mar 1, 2022
  59. 7/9 ci: optionally mark up output in the GitHub workflowJohannes Schindelin via GitGitGadget, Mar 1, 2022
  60. 8/9 ci: use `--github-workflow-markup` in the GitHub workflowJohannes Schindelin via GitGitGadget, Mar 1, 2022
  61. 9/9 ci: call `finalize_test_case_output` a little laterJohannes Schindelin via GitGitGadget, Mar 1, 2022
  62. Junio C HamanoMar 1, 2022
  63. Ævar Arnfjörð BjarmasonMar 2, 2022
  64. Johannes SchindelinMar 7, 2022
  65. Ævar Arnfjörð BjarmasonMar 7, 2022
  66. Junio C HamanoMar 7, 2022
  67. Ævar Arnfjörð BjarmasonMar 9, 2022
  68. Johannes SchindelinMar 9, 2022
  69. Junio C HamanoMar 9, 2022
  70. Ævar Arnfjörð BjarmasonMar 9, 2022
  71. Victoria DyeMar 25, 2022
  72. Ævar Arnfjörð BjarmasonMar 25, 2022
  73. Victoria DyeMar 25, 2022
  74. Johannes SchindelinMay 21, 2022
  75. Junio C HamanoMay 21, 2022
  76. Johannes SchindelinMay 22, 2022
  77. Junio C HamanoMay 22, 2022
  78. Johannes SchindelinMay 23, 2022
  79. Junio C HamanoMay 22, 2022
  80. Junio C HamanoMay 23, 2022
  81. Johannes SchindelinMay 23, 2022
  82. Ævar Arnfjörð BjarmasonMay 23, 2022
  83. Johannes SchindelinMay 23, 2022
  84. Ævar Arnfjörð BjarmasonMay 24, 2022
  85. 00/12 ci: make Git's GitHub workflow output much more helpfulJohannes Schindelin via GitGitGadget, May 21, 2022
  86. 01/12 ci: fix code styleJohannes Schindelin via GitGitGadget, May 21, 2022
  87. 02/12 tests: refactor --write-junit-xml codeJohannes Schindelin via GitGitGadget, May 21, 2022
  88. 03/12 test(junit): avoid line feeds in XML attributesJohannes Schindelin via GitGitGadget, May 21, 2022
  89. 04/12 ci/run-build-and-tests: take a more high-level viewJohannes Schindelin via GitGitGadget, May 21, 2022
  90. 05/12 ci: make it easier to find failed tests' logs in the GitHub workflowJohannes Schindelin via GitGitGadget, May 21, 2022
  91. 06/12 ci/run-build-and-tests: add some structure to the GitHub workflow outputJohannes Schindelin via GitGitGadget, May 21, 2022
  92. 07/12 ci: optionally mark up output in the GitHub workflowJohannes Schindelin via GitGitGadget, May 21, 2022
  93. 08/12 ci(github): skip the logs of the successful test casesJohannes Schindelin via GitGitGadget, May 21, 2022
  94. Ævar Arnfjörð BjarmasonMay 24, 2022
  95. 10/12 ci: use `--github-workflow-markup` in the GitHub workflowJohannes Schindelin via GitGitGadget, May 21, 2022
  96. 09/12 ci(github): avoid printing test case preamble twiceVictoria Dye via GitGitGadget, May 21, 2022
  97. 12/12 ci: call `finalize_test_case_output` a little laterJohannes Schindelin via GitGitGadget, May 21, 2022
  98. 11/12 ci(github): mention where the full logs can be foundJohannes Schindelin via GitGitGadget, May 21, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.