tests: add 'test_bool_env' to catch non-bool GIT_TEST_* values
Since 3b072c577b (tests: replace test_tristate with "git env--helper", 2019-06-21) we get the normalized bool values of various GIT_TEST_* environment variables via 'git env--helper'. Now, while the 'git env--helper' command itself does catch invalid values in the environment variable or in the given --default and exits with error (exit code 128 or 129, respectively), it's invoked in conditions like 'if ! git env--helper ...', which means that all invalid bool values are interpreted the same as the ordinary 'false' (exit code 1). This has led to inadvertently skipped httpd tests in our CI builds for a couple of weeks, see 3960290675 (ci: restore running httpd tests, 2019-09-06). Let's be more careful about what the test suite accepts as bool values in GIT_TEST_* environment variables, and error out loud and clear on invalid values instead of simply skipping tests. Add the 'test_bool_env' helper function to encapsulate the invocation of 'git env--helper' and the verification of its exit code, and replace all invocations of that command in our test framework and test suite with a call to this new helper (except in 't0017-env-helper.sh', of course). $ GIT_TEST_GIT_DAEMON=YesPlease ./t5570-git-daemon.sh fatal: bad numeric config value 'YesPlease' for 'GIT_TEST_GIT_DAEMON': invalid unit error: test_bool_env requires bool values both for $GIT_TEST_GIT_DAEMON and for the default fallback Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor committed
Nov 22, 2019 at 14:14 UTC
43a2afee82acd0c51609a7840ca71ac5291020b4
8 files changed
+82
-11
t/README
+9
@@ -978,6 +978,15 @@ library for your script to use.
978
output to the downstream---unlike the real version, it generates
979
only up to 99 lines.
980
981
+ - test_bool_env <env-variable-name> <default-value>
982
+
983
+ Given the name of an environment variable with a bool value,
984
+ normalize its value to a 0 (true) or 1 (false or empty string)
985
+ return code. Return with code corresponding to the given default
986
+ value if the variable is unset.
987
+ Abort the test script if either the value of the variable or the
988
+ default are not valid bool values.
989
+
990
991
Prerequisites
992
-------------
t/lib-git-daemon.sh
+1
-1
@@ -15,7 +15,7 @@
15
#
16
# test_done
17
18
-if ! git env--helper --type=bool --default=true --exit-code GIT_TEST_GIT_DAEMON
18
+if ! test_bool_env GIT_TEST_GIT_DAEMON true
19
then
20
skip_all="git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to enable)"
21
test_done
t/lib-git-svn.sh
+2
-2
@@ -69,7 +69,7 @@ svn_cmd () {
69
maybe_start_httpd () {
70
loc=${1-svn}
71
72
- if git env--helper --type=bool --default=false --exit-code GIT_TEST_SVN_HTTPD
72
+ if test_bool_env GIT_TEST_SVN_HTTPD false
73
then
74
. "$TEST_DIRECTORY"/lib-httpd.sh
75
LIB_HTTPD_SVN="$loc"
@@ -104,7 +104,7 @@ EOF
104
}
105
106
require_svnserve () {
107
- if ! git env--helper --type=bool --default=false --exit-code GIT_TEST_SVNSERVE
107
+ if ! test_bool_env GIT_TEST_SVNSERVE false
108
then
109
skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to enable)'
110
test_done
t/lib-httpd.sh
+1
-1
@@ -41,7 +41,7 @@ then
41
test_done
42
fi
43
44
-if ! git env--helper --type=bool --default=true --exit-code GIT_TEST_HTTPD
44
+if ! test_bool_env GIT_TEST_HTTPD true
45
then
46
skip_all="Network testing disabled (unset GIT_TEST_HTTPD to enable)"
47
test_done
t/t0000-basic.sh
+34
@@ -916,6 +916,40 @@ test_expect_success 'test_oid can look up data for SHA-256' '
916
test "$hexsz" -eq 64
917
'
918
919
+test_expect_success 'test_bool_env' '
920
+ (
921
+ sane_unset envvar &&
922
+
923
+ test_bool_env envvar true &&
924
+ ! test_bool_env envvar false &&
925
+
926
+ envvar= &&
927
+ export envvar &&
928
+ ! test_bool_env envvar true &&
929
+ ! test_bool_env envvar false &&
930
+
931
+ envvar=true &&
932
+ test_bool_env envvar true &&
933
+ test_bool_env envvar false &&
934
+
935
+ envvar=false &&
936
+ ! test_bool_env envvar true &&
937
+ ! test_bool_env envvar false &&
938
+
939
+ envvar=invalid &&
940
+ # When encountering an invalid bool value, test_bool_env
941
+ # prints its error message to the original stderr of the
942
+ # test script, hence the redirection of fd 7, and aborts
943
+ # with "exit 1", hence the subshell.
944
+ ! ( test_bool_env envvar true ) 7>err &&
945
+ grep "error: test_bool_env requires bool values" err &&
946
+
947
+ envvar=true &&
948
+ ! ( test_bool_env envvar invalid ) 7>err &&
949
+ grep "error: test_bool_env requires bool values" err
950
+ )
951
+'
952
+
953
################################################################
954
# Basics of the basics
955
t/t5512-ls-remote.sh
+1
-1
@@ -267,7 +267,7 @@ test_expect_success 'ls-remote --symref omits filtered-out matches' '
267
'
268
269
test_lazy_prereq GIT_DAEMON '
270
- git env--helper --type=bool --default=true --exit-code GIT_TEST_GIT_DAEMON
270
+ test_bool_env GIT_TEST_GIT_DAEMON true
271
'
272
273
# This test spawns a daemon, so run it only if the user would be OK with
t/test-lib-functions.sh
+29
-1
@@ -1175,6 +1175,34 @@ perl () {
1175
command "$PERL_PATH" "$@" 2>&7
1176
} 7>&2 2>&4
1177
1178
+# Given the name of an environment variable with a bool value, normalize
1179
+# its value to a 0 (true) or 1 (false or empty string) return code.
1180
+#
1181
+# test_bool_env GIT_TEST_HTTPD <default-value>
1182
+#
1183
+# Return with code corresponding to the given default value if the variable
1184
+# is unset.
1185
+# Abort the test script if either the value of the variable or the default
1186
+# are not valid bool values.
1187
+
1188
+test_bool_env () {
1189
+ if test $# != 2
1190
+ then
1191
+ BUG "test_bool_env requires two parameters (variable name and default value)"
1192
+ fi
1193
+
1194
+ git env--helper --type=bool --default="$2" --exit-code "$1"
1195
+ ret=$?
1196
+ case $ret in
1197
+ 0|1) # unset or valid bool value
1198
+ ;;
1199
+ *) # invalid bool value or something unexpected
1200
+ error >&7 "test_bool_env requires bool values both for \$$1 and for the default fallback"
1201
+ ;;
1202
+ esac
1203
+ return $ret
1204
+}
1205
+
1206
# Exit the test suite, either by skipping all remaining tests or by
1207
# exiting with an error. If our prerequisite variable $1 falls back
1208
# on a default assume we were opportunistically trying to set up some
@@ -1183,7 +1211,7 @@ perl () {
1211
# The error/skip message should be given by $2.
1212
#
1213
test_skip_or_die () {
1186
- if ! git env--helper --type=bool --default=false --exit-code $1
1214
+ if ! test_bool_env "$1" false
1215
then
1216
skip_all=$2
1217
test_done
t/test-lib.sh
+5
-5
@@ -1406,19 +1406,19 @@ yes () {
1406
# The GIT_TEST_FAIL_PREREQS code hooks into test_set_prereq(), and
1407
# thus needs to be set up really early, and set an internal variable
1408
# for convenience so the hot test_set_prereq() codepath doesn't need
1409
-# to call "git env--helper". Only do that work if needed by seeing if
1410
-# GIT_TEST_FAIL_PREREQS is set at all.
1409
+# to call "git env--helper" (via test_bool_env). Only do that work
1410
+# if needed by seeing if GIT_TEST_FAIL_PREREQS is set at all.
1411
GIT_TEST_FAIL_PREREQS_INTERNAL=
1412
if test -n "$GIT_TEST_FAIL_PREREQS"
1413
then
1414
- if git env--helper --type=bool --default=0 --exit-code GIT_TEST_FAIL_PREREQS
1414
+ if test_bool_env GIT_TEST_FAIL_PREREQS false
1415
then
1416
GIT_TEST_FAIL_PREREQS_INTERNAL=true
1417
test_set_prereq FAIL_PREREQS
1418
fi
1419
else
1420
test_lazy_prereq FAIL_PREREQS '
1421
- git env--helper --type=bool --default=0 --exit-code GIT_TEST_FAIL_PREREQS
1421
+ test_bool_env GIT_TEST_FAIL_PREREQS false
1422
'
1423
fi
1424
@@ -1477,7 +1477,7 @@ then
1477
fi
1478
1479
test_lazy_prereq C_LOCALE_OUTPUT '
1480
- ! git env--helper --type=bool --default=0 --exit-code GIT_TEST_GETTEXT_POISON
1480
+ ! test_bool_env GIT_TEST_GETTEXT_POISON false
1481
'
1482
1483
if test -z "$GIT_TEST_CHECK_CACHE_TREE"