Skip to content

Commit 78d5e4c

Browse files
dschogitster
dscho
authored and
gitster
committed
tests: refactor --write-junit-xml code
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 863d6ce commit 78d5e4c

File tree

2 files changed

+142
-108
lines changed

2 files changed

+142
-108
lines changed

‎t/test-lib-junit.sh

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
# Library of functions to format test scripts' output in JUnit XML
2+
# format, to support Git's test suite result to be presented in an
3+
# easily digestible way on Azure Pipelines.
4+
#
5+
# Copyright (c) 2022 Johannes Schindelin
6+
#
7+
# This program is free software: you can redistribute it and/or modify
8+
# it under the terms of the GNU General Public License as published by
9+
# the Free Software Foundation, either version 2 of the License, or
10+
# (at your option) any later version.
11+
#
12+
# This program is distributed in the hope that it will be useful,
13+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
# GNU General Public License for more details.
16+
#
17+
# You should have received a copy of the GNU General Public License
18+
# along with this program. If not, see http://www.gnu.org/licenses/ .
19+
#
20+
# The idea is for `test-lib.sh` to source this file when the user asks
21+
# for JUnit XML; these functions will then override (empty) functions
22+
# that are are called at the appropriate times during the test runs.
23+
24+
start_test_output () {
25+
junit_xml_dir="$TEST_OUTPUT_DIRECTORY/out"
26+
mkdir -p "$junit_xml_dir"
27+
junit_xml_base=${1##*/}
28+
junit_xml_path="$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml"
29+
junit_attrs="name=\"${junit_xml_base%.sh}\""
30+
junit_attrs="$junit_attrs timestamp=\"$(TZ=UTC \
31+
date +%Y-%m-%dT%H:%M:%S)\""
32+
write_junit_xml --truncate "<testsuites>" " <testsuite $junit_attrs>"
33+
junit_suite_start=$(test-tool date getnanos)
34+
if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
35+
then
36+
GIT_TEST_TEE_OFFSET=0
37+
fi
38+
}
39+
40+
start_test_case_output () {
41+
junit_start=$(test-tool date getnanos)
42+
}
43+
44+
finalize_test_case_output () {
45+
test_case_result=$1
46+
shift
47+
case "$test_case_result" in
48+
ok)
49+
set "$*"
50+
;;
51+
failure)
52+
junit_insert="<failure message=\"not ok $test_count -"
53+
junit_insert="$junit_insert $(xml_attr_encode "$1")\">"
54+
junit_insert="$junit_insert $(xml_attr_encode \
55+
"$(if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
56+
then
57+
test-tool path-utils skip-n-bytes \
58+
"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
59+
else
60+
printf '%s\n' "$@" | sed 1d
61+
fi)")"
62+
junit_insert="$junit_insert</failure>"
63+
if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
64+
then
65+
junit_insert="$junit_insert<system-err>$(xml_attr_encode \
66+
"$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")</system-err>"
67+
fi
68+
set "$1" " $junit_insert"
69+
;;
70+
fixed)
71+
set "$* (breakage fixed)"
72+
;;
73+
broken)
74+
set "$* (known breakage)"
75+
;;
76+
skip)
77+
message="$(xml_attr_encode "$skipped_reason")"
78+
set "$1" " <skipped message=\"$message\" />"
79+
;;
80+
esac
81+
82+
junit_attrs="name=\"$(xml_attr_encode "$this_test.$test_count $1")\""
83+
shift
84+
junit_attrs="$junit_attrs classname=\"$this_test\""
85+
junit_attrs="$junit_attrs time=\"$(test-tool \
86+
date getnanos $junit_start)\""
87+
write_junit_xml "$(printf '%s\n' \
88+
" <testcase $junit_attrs>" "$@" " </testcase>")"
89+
junit_have_testcase=t
90+
}
91+
92+
finalize_test_output () {
93+
if test -n "$junit_xml_path"
94+
then
95+
test -n "$junit_have_testcase" || {
96+
junit_start=$(test-tool date getnanos)
97+
write_junit_xml_testcase "all tests skipped"
98+
}
99+
100+
# adjust the overall time
101+
junit_time=$(test-tool date getnanos $junit_suite_start)
102+
sed -e "s/\(<testsuite.*\) time=\"[^\"]*\"/\1/" \
103+
-e "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
104+
-e '/^ *<\/testsuite/d' \
105+
<"$junit_xml_path" >"$junit_xml_path.new"
106+
mv "$junit_xml_path.new" "$junit_xml_path"
107+
108+
write_junit_xml " </testsuite>" "</testsuites>"
109+
write_junit_xml=
110+
fi
111+
}
112+
113+
write_junit_xml () {
114+
case "$1" in
115+
--truncate)
116+
>"$junit_xml_path"
117+
junit_have_testcase=
118+
shift
119+
;;
120+
esac
121+
printf '%s\n' "$@" >>"$junit_xml_path"
122+
}
123+
124+
xml_attr_encode () {
125+
printf '%s\n' "$@" | test-tool xml-encode
126+
}

‎t/test-lib.sh

Lines changed: 16 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ mark_option_requires_arg () {
137137
store_arg_to=$2
138138
}
139139

140+
# These functions can be overridden e.g. to output JUnit XML
141+
start_test_output () { :; }
142+
start_test_case_output () { :; }
143+
finalize_test_case_output () { :; }
144+
finalize_test_output () { :; }
145+
140146
parse_option () {
141147
local opt="$1"
142148

@@ -196,7 +202,7 @@ parse_option () {
196202
tee=t
197203
;;
198204
--write-junit-xml)
199-
write_junit_xml=t
205+
. "$TEST_DIRECTORY/test-lib-junit.sh"
200206
;;
201207
--stress)
202208
stress=t ;;
@@ -664,7 +670,7 @@ exec 6<&0
664670
exec 7>&2
665671

666672
_error_exit () {
667-
finalize_junit_xml
673+
finalize_test_output
668674
GIT_EXIT_OK=t
669675
exit 1
670676
}
@@ -774,35 +780,13 @@ trap '{ code=$?; set +x; } 2>/dev/null; exit $code' INT TERM HUP
774780
# the test_expect_* functions instead.
775781

776782
test_ok_ () {
777-
if test -n "$write_junit_xml"
778-
then
779-
write_junit_xml_testcase "$*"
780-
fi
783+
finalize_test_case_output ok "$@"
781784
test_success=$(($test_success + 1))
782785
say_color "" "ok $test_count - $@"
783786
}
784787

785788
test_failure_ () {
786-
if test -n "$write_junit_xml"
787-
then
788-
junit_insert="<failure message=\"not ok $test_count -"
789-
junit_insert="$junit_insert $(xml_attr_encode "$1")\">"
790-
junit_insert="$junit_insert $(xml_attr_encode \
791-
"$(if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
792-
then
793-
test-tool path-utils skip-n-bytes \
794-
"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
795-
else
796-
printf '%s\n' "$@" | sed 1d
797-
fi)")"
798-
junit_insert="$junit_insert</failure>"
799-
if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
800-
then
801-
junit_insert="$junit_insert<system-err>$(xml_attr_encode \
802-
"$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")</system-err>"
803-
fi
804-
write_junit_xml_testcase "$1" " $junit_insert"
805-
fi
789+
finalize_test_case_output failure "$@"
806790
test_failure=$(($test_failure + 1))
807791
say_color error "not ok $test_count - $1"
808792
shift
@@ -815,19 +799,13 @@ test_failure_ () {
815799
}
816800

817801
test_known_broken_ok_ () {
818-
if test -n "$write_junit_xml"
819-
then
820-
write_junit_xml_testcase "$* (breakage fixed)"
821-
fi
802+
finalize_test_case_output fixed "$@"
822803
test_fixed=$(($test_fixed+1))
823804
say_color error "ok $test_count - $@ # TODO known breakage vanished"
824805
}
825806

826807
test_known_broken_failure_ () {
827-
if test -n "$write_junit_xml"
828-
then
829-
write_junit_xml_testcase "$* (known breakage)"
830-
fi
808+
finalize_test_case_output broken "$@"
831809
test_broken=$(($test_broken+1))
832810
say_color warn "not ok $test_count - $@ # TODO known breakage"
833811
}
@@ -1104,10 +1082,7 @@ test_start_ () {
11041082
test_count=$(($test_count+1))
11051083
maybe_setup_verbose
11061084
maybe_setup_valgrind
1107-
if test -n "$write_junit_xml"
1108-
then
1109-
junit_start=$(test-tool date getnanos)
1110-
fi
1085+
start_test_case_output
11111086
}
11121087

11131088
test_finish_ () {
@@ -1158,12 +1133,7 @@ test_skip () {
11581133

11591134
case "$to_skip" in
11601135
t)
1161-
if test -n "$write_junit_xml"
1162-
then
1163-
message="$(xml_attr_encode "$skipped_reason")"
1164-
write_junit_xml_testcase "$1" \
1165-
" <skipped message=\"$message\" />"
1166-
fi
1136+
finalize_test_case_output skip "$@"
11671137

11681138
say_color skip "ok $test_count # skip $1 ($skipped_reason)"
11691139
: true
@@ -1179,53 +1149,6 @@ test_at_end_hook_ () {
11791149
:
11801150
}
11811151

1182-
write_junit_xml () {
1183-
case "$1" in
1184-
--truncate)
1185-
>"$junit_xml_path"
1186-
junit_have_testcase=
1187-
shift
1188-
;;
1189-
esac
1190-
printf '%s\n' "$@" >>"$junit_xml_path"
1191-
}
1192-
1193-
xml_attr_encode () {
1194-
printf '%s\n' "$@" | test-tool xml-encode
1195-
}
1196-
1197-
write_junit_xml_testcase () {
1198-
junit_attrs="name=\"$(xml_attr_encode "$this_test.$test_count $1")\""
1199-
shift
1200-
junit_attrs="$junit_attrs classname=\"$this_test\""
1201-
junit_attrs="$junit_attrs time=\"$(test-tool \
1202-
date getnanos $junit_start)\""
1203-
write_junit_xml "$(printf '%s\n' \
1204-
" <testcase $junit_attrs>" "$@" " </testcase>")"
1205-
junit_have_testcase=t
1206-
}
1207-
1208-
finalize_junit_xml () {
1209-
if test -n "$write_junit_xml" && test -n "$junit_xml_path"
1210-
then
1211-
test -n "$junit_have_testcase" || {
1212-
junit_start=$(test-tool date getnanos)
1213-
write_junit_xml_testcase "all tests skipped"
1214-
}
1215-
1216-
# adjust the overall time
1217-
junit_time=$(test-tool date getnanos $junit_suite_start)
1218-
sed -e "s/\(<testsuite.*\) time=\"[^\"]*\"/\1/" \
1219-
-e "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
1220-
-e '/^ *<\/testsuite/d' \
1221-
<"$junit_xml_path" >"$junit_xml_path.new"
1222-
mv "$junit_xml_path.new" "$junit_xml_path"
1223-
1224-
write_junit_xml " </testsuite>" "</testsuites>"
1225-
write_junit_xml=
1226-
fi
1227-
}
1228-
12291152
test_atexit_cleanup=:
12301153
test_atexit_handler () {
12311154
# In a succeeding test script 'test_atexit_handler' is invoked
@@ -1248,7 +1171,7 @@ test_done () {
12481171
# removed, so the commands can access pidfiles and socket files.
12491172
test_atexit_handler
12501173

1251-
finalize_junit_xml
1174+
finalize_test_output
12521175

12531176
if test -z "$HARNESS_ACTIVE"
12541177
then
@@ -1539,22 +1462,7 @@ fi
15391462
# in subprocesses like git equals our $PWD (for pathname comparisons).
15401463
cd -P "$TRASH_DIRECTORY" || exit 1
15411464

1542-
if test -n "$write_junit_xml"
1543-
then
1544-
junit_xml_dir="$TEST_OUTPUT_DIRECTORY/out"
1545-
mkdir -p "$junit_xml_dir"
1546-
junit_xml_base=${0##*/}
1547-
junit_xml_path="$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml"
1548-
junit_attrs="name=\"${junit_xml_base%.sh}\""
1549-
junit_attrs="$junit_attrs timestamp=\"$(TZ=UTC \
1550-
date +%Y-%m-%dT%H:%M:%S)\""
1551-
write_junit_xml --truncate "<testsuites>" " <testsuite $junit_attrs>"
1552-
junit_suite_start=$(test-tool date getnanos)
1553-
if test -n "$GIT_TEST_TEE_OUTPUT_FILE"
1554-
then
1555-
GIT_TEST_TEE_OFFSET=0
1556-
fi
1557-
fi
1465+
start_test_output "$0"
15581466

15591467
# Convenience
15601468
# A regexp to match 5 and 35 hexdigits

0 commit comments

Comments
 (0)