Re: [PATCH] tests: turn on network daemon tests by default

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2014, 19:51 UTC
Message-ID: <xmqqa9dxpgw9.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140210212931.GA16154@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> I dug in the history to see if there was any reasoning given for the
> current "off by default" setting. It looks like Junio asked for it when
> the original http-push tests were added, and everything else just
> followed that. The reasoning there was basically "they're heavyweight
> and we might not be able to run them", but hopefully having the option
> variable will make that OK.

Will queue, thanks.

I may have originally asked for it for one reason, thinking that one reason would be enough while having another reason not to run them as well. But there would be countless silent others who have been depending on that choice.

Those who run buildfarms may want to disable the networking test if the buildfarms are not isolated well, for example. They have to be told somewhere that now they need to explicitly disable these tests and how.

I am in favor of this change but just pointing out possible fallouts might be larger than we think.

Show 150 quoted lines

>  t/lib-git-daemon.sh     |  8 +++++---
>  t/lib-httpd.sh          | 22 +++++++++++-----------
>  t/test-lib-functions.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 60 insertions(+), 14 deletions(-)
>
> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
> index 1f22de2..e623bef 100644
> --- a/t/lib-git-daemon.sh
> +++ b/t/lib-git-daemon.sh
> @@ -16,9 +16,10 @@
>  #	stop_git_daemon
>  #	test_done
>  
> -if test -z "$GIT_TEST_GIT_DAEMON"
> +GIT_TEST_GIT_DAEMON=$(test_tristate "$GIT_TEST_GIT_DAEMON")
> +if test "$GIT_TEST_GIT_DAEMON" = false
>  then
> -	skip_all="git-daemon testing disabled (define GIT_TEST_GIT_DAEMON to enable)"
> +	skip_all="git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to enable)"
>  	test_done
>  fi
>  
> @@ -58,7 +59,8 @@ start_git_daemon() {
>  		kill "$GIT_DAEMON_PID"
>  		wait "$GIT_DAEMON_PID"
>  		trap 'die' EXIT
> -		error "git daemon failed to start"
> +		test_skip_or_die $GIT_TEST_GIT_DAEMON \
> +			"git daemon failed to start"
>  	fi
>  }
>  
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index b43162e..bb900ca 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -30,9 +30,10 @@
>  # Copyright (c) 2008 Clemens Buchacher <drizzd@aon.at>
>  #
>  
> -if test -z "$GIT_TEST_HTTPD"
> +GIT_TEST_HTTPD=$(test_tristate "$GIT_TEST_HTTPD")
> +if test "$GIT_TEST_HTTPD" = false
>  then
> -	skip_all="Network testing disabled (define GIT_TEST_HTTPD to enable)"
> +	skip_all="Network testing disabled (unset GIT_TEST_HTTPD to enable)"
>  	test_done
>  fi
>  
> @@ -76,8 +77,7 @@ GIT_VALGRIND_OPTIONS=$GIT_VALGRIND_OPTIONS; export GIT_VALGRIND_OPTIONS
>  
>  if ! test -x "$LIB_HTTPD_PATH"
>  then
> -	skip_all="skipping test, no web server found at '$LIB_HTTPD_PATH'"
> -	test_done
> +	test_skip_or_die $GIT_TEST_HTTPD "no web server found at '$LIB_HTTPD_PATH'"
>  fi
>  
>  HTTPD_VERSION=`$LIB_HTTPD_PATH -v | \
> @@ -89,19 +89,20 @@ then
>  	then
>  		if ! test $HTTPD_VERSION -ge 2
>  		then
> -			skip_all="skipping test, at least Apache version 2 is required"
> -			test_done
> +			test_skip_or_die $GIT_TEST_HTTPD \
> +				"at least Apache version 2 is required"
>  		fi
>  		if ! test -d "$DEFAULT_HTTPD_MODULE_PATH"
>  		then
> -			skip_all="Apache module directory not found.  Skipping tests."
> -			test_done
> +			test_skip_or_die $GIT_TEST_HTTPD \
> +				"Apache module directory not found"
>  		fi
>  
>  		LIB_HTTPD_MODULE_PATH="$DEFAULT_HTTPD_MODULE_PATH"
>  	fi
>  else
> -	error "Could not identify web server at '$LIB_HTTPD_PATH'"
> +	test_skip_or_die $GIT_TEST_HTTPD \
> +		"Could not identify web server at '$LIB_HTTPD_PATH'"
>  fi
>  
>  prepare_httpd() {
> @@ -155,9 +156,8 @@ start_httpd() {
>  		>&3 2>&4
>  	if test $? -ne 0
>  	then
> -		skip_all="skipping test, web server setup failed"
>  		trap 'die' EXIT
> -		test_done
> +		test_skip_or_die $GIT_TEST_HTTPD "web server setup failed"
>  	fi
>  }
>  
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index aeae3ca..3cc09c0 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -716,6 +716,50 @@ perl () {
>  	command "$PERL_PATH" "$@"
>  }
>  
> +# Normalize the value given in $1 to one of "true", "false", or "auto". The
> +# result is written to stdout. E.g.:
> +#
> +#     GIT_TEST_HTTPD=$(test_tristate "$GIT_TEST_HTTPD")
> +#
> +test_tristate () {
> +	case "$1" in
> +	""|auto)
> +		echo auto
> +		;;
> +	*)
> +		# Rely on git-config to handle all the variants of
> +		# true/1/on/etc that we allow.
> +		if ! git -c magic.hack="$1" config --bool magic.hack 2>/dev/null
> +		then
> +			# If git-config failed, it was some non-bool value like
> +			# YesPlease. Default this to "true" for historical
> +			# compatibility.
> +			echo true
> +		fi
> +	esac
> +}
> +
> +# Exit the test suite, either by skipping all remaining tests or by
> +# exiting with an error. If "$1" is "auto", we then we assume we were
> +# opportunistically trying to set up some tests and we skip. If it is
> +# "true", then we report a failure.
> +#
> +# The error/skip message should be given by $2.
> +#
> +test_skip_or_die () {
> +	case "$1" in
> +	auto)
> +		skip_all=$2
> +		test_done
> +		;;
> +	true)
> +		error "$2"
> +		;;
> +	*)
> +		error "BUG: test tristate is '$1' (real error: $2)"
> +	esac
> +}
> +
>  # The following mingw_* functions obey POSIX shell syntax, but are actually
>  # bash scripts, and are meant to be used only with bash on Windows.