Merge branch 'sg/test-bool-env'

Recently we have declared that GIT_TEST_* variables take the
usual boolean values (it used to be that some used "non-empty
means true" and taking GIT_TEST_VAR=YesPlease as true); make
sure we notice and fail when non-bool strings are given to
these variables.

* sg/test-bool-env:
  t5608-clone-2gb.sh: turn GIT_TEST_CLONE_2GB into a bool
  tests: add 'test_bool_env' to catch non-bool GIT_TEST_* values

10 files changed, 84 insertions, 13 deletions

diff --git a/ci/lib.sh b/ci/lib.sh
index c8c2c38155..5fca0438b5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -156,7 +156,7 @@ fi
 
 export DEVELOPER=1
 export DEFAULT_TEST_TARGET=prove
-export GIT_TEST_CLONE_2GB=YesPlease
+export GIT_TEST_CLONE_2GB=true
 
 case "$jobname" in
 linux-clang|linux-gcc)
diff --git a/t/README b/t/README
index 5132ec83f8..caa125ba9a 100644
--- a/t/README
+++ b/t/README
@@ -982,6 +982,15 @@ library for your script to use.
    output to the downstream---unlike the real version, it generates
    only up to 99 lines.
 
+ - test_bool_env <env-variable-name> <default-value>
+
+   Given the name of an environment variable with a bool value,
+   normalize its value to a 0 (true) or 1 (false or empty string)
+   return code.  Return with code corresponding to the given default
+   value if the variable is unset.
+   Abort the test script if either the value of the variable or the
+   default are not valid bool values.
+
 
 Prerequisites
 -------------
diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index fb8f887080..e62569222b 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -15,7 +15,7 @@
 #
 #	test_done
 
-if ! git env--helper --type=bool --default=true --exit-code GIT_TEST_GIT_DAEMON
+if ! test_bool_env GIT_TEST_GIT_DAEMON true
 then
 	skip_all="git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to enable)"
 	test_done
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index bc0b9c71f8..7d248e6588 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -69,7 +69,7 @@ svn_cmd () {
 maybe_start_httpd () {
 	loc=${1-svn}
 
-	if git env--helper --type=bool --default=false --exit-code GIT_TEST_SVN_HTTPD
+	if test_bool_env GIT_TEST_SVN_HTTPD false
 	then
 		. "$TEST_DIRECTORY"/lib-httpd.sh
 		LIB_HTTPD_SVN="$loc"
@@ -104,7 +104,7 @@ EOF
 }
 
 require_svnserve () {
-	if ! git env--helper --type=bool --default=false --exit-code GIT_TEST_SVNSERVE
+	if ! test_bool_env GIT_TEST_SVNSERVE false
 	then
 		skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to enable)'
 		test_done
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 0d985758c6..656997b4d6 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -41,7 +41,7 @@ then
 	test_done
 fi
 
-if ! git env--helper --type=bool --default=true --exit-code GIT_TEST_HTTPD
+if ! test_bool_env GIT_TEST_HTTPD true
 then
 	skip_all="Network testing disabled (unset GIT_TEST_HTTPD to enable)"
 	test_done
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index a5195a8dc4..8a81a249d0 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -917,6 +917,40 @@ test_expect_success 'test_oid can look up data for SHA-256' '
 	test "$hexsz" -eq 64
 '
 
+test_expect_success 'test_bool_env' '
+	(
+		sane_unset envvar &&
+
+		test_bool_env envvar true &&
+		! test_bool_env envvar false &&
+
+		envvar= &&
+		export envvar &&
+		! test_bool_env envvar true &&
+		! test_bool_env envvar false &&
+
+		envvar=true &&
+		test_bool_env envvar true &&
+		test_bool_env envvar false &&
+
+		envvar=false &&
+		! test_bool_env envvar true &&
+		! test_bool_env envvar false &&
+
+		envvar=invalid &&
+		# When encountering an invalid bool value, test_bool_env
+		# prints its error message to the original stderr of the
+		# test script, hence the redirection of fd 7, and aborts
+		# with "exit 1", hence the subshell.
+		! ( test_bool_env envvar true ) 7>err &&
+		grep "error: test_bool_env requires bool values" err &&
+
+		envvar=true &&
+		! ( test_bool_env envvar invalid ) 7>err &&
+		grep "error: test_bool_env requires bool values" err
+	)
+'
+
 ################################################################
 # Basics of the basics
 
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 43e1d8d4d2..d7b9f9078f 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -267,7 +267,7 @@ test_expect_success 'ls-remote --symref omits filtered-out matches' '
 '
 
 test_lazy_prereq GIT_DAEMON '
-	git env--helper --type=bool --default=true --exit-code GIT_TEST_GIT_DAEMON
+	test_bool_env GIT_TEST_GIT_DAEMON true
 '
 
 # This test spawns a daemon, so run it only if the user would be OK with
diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index 2c6bc07344..eee0842888 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -3,7 +3,7 @@
 test_description='Test cloning a repository larger than 2 gigabyte'
 . ./test-lib.sh
 
-if test -z "$GIT_TEST_CLONE_2GB"
+if ! test_bool_env GIT_TEST_CLONE_2GB false
 then
 	say 'Skipping expensive 2GB clone test; enable it with GIT_TEST_CLONE_2GB=t'
 else
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index efcd96fc9e..dc7a0411df 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1186,6 +1186,34 @@ perl () {
 	command "$PERL_PATH" "$@" 2>&7
 } 7>&2 2>&4
 
+# Given the name of an environment variable with a bool value, normalize
+# its value to a 0 (true) or 1 (false or empty string) return code.
+#
+#   test_bool_env GIT_TEST_HTTPD <default-value>
+#
+# Return with code corresponding to the given default value if the variable
+# is unset.
+# Abort the test script if either the value of the variable or the default
+# are not valid bool values.
+
+test_bool_env () {
+	if test $# != 2
+	then
+		BUG "test_bool_env requires two parameters (variable name and default value)"
+	fi
+
+	git env--helper --type=bool --default="$2" --exit-code "$1"
+	ret=$?
+	case $ret in
+	0|1)	# unset or valid bool value
+		;;
+	*)	# invalid bool value or something unexpected
+		error >&7 "test_bool_env requires bool values both for \$$1 and for the default fallback"
+		;;
+	esac
+	return $ret
+}
+
 # Exit the test suite, either by skipping all remaining tests or by
 # exiting with an error. If our prerequisite variable $1 falls back
 # on a default assume we were opportunistically trying to set up some
@@ -1194,7 +1222,7 @@ perl () {
 # The error/skip message should be given by $2.
 #
 test_skip_or_die () {
-	if ! git env--helper --type=bool --default=false --exit-code $1
+	if ! test_bool_env "$1" false
 	then
 		skip_all=$2
 		test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e8ae150d75..5fbf4d25ed 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1406,19 +1406,19 @@ yes () {
 # The GIT_TEST_FAIL_PREREQS code hooks into test_set_prereq(), and
 # thus needs to be set up really early, and set an internal variable
 # for convenience so the hot test_set_prereq() codepath doesn't need
-# to call "git env--helper". Only do that work if needed by seeing if
-# GIT_TEST_FAIL_PREREQS is set at all.
+# to call "git env--helper" (via test_bool_env). Only do that work
+# if needed by seeing if GIT_TEST_FAIL_PREREQS is set at all.
 GIT_TEST_FAIL_PREREQS_INTERNAL=
 if test -n "$GIT_TEST_FAIL_PREREQS"
 then
-	if git env--helper --type=bool --default=0 --exit-code GIT_TEST_FAIL_PREREQS
+	if test_bool_env GIT_TEST_FAIL_PREREQS false
 	then
 		GIT_TEST_FAIL_PREREQS_INTERNAL=true
 		test_set_prereq FAIL_PREREQS
 	fi
 else
 	test_lazy_prereq FAIL_PREREQS '
-		git env--helper --type=bool --default=0 --exit-code GIT_TEST_FAIL_PREREQS
+		test_bool_env GIT_TEST_FAIL_PREREQS false
 	'
 fi
 
@@ -1477,7 +1477,7 @@ then
 fi
 
 test_lazy_prereq C_LOCALE_OUTPUT '
-	! git env--helper --type=bool --default=0 --exit-code GIT_TEST_GETTEXT_POISON
+	! test_bool_env GIT_TEST_GETTEXT_POISON false
 '
 
 if test -z "$GIT_TEST_CHECK_CACHE_TREE"