All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>,
	"Shawn O. Pearce" <spearce@spearce.org>
Subject: [PATCH] tests: turn on network daemon tests by default
Date: Mon, 10 Feb 2014 16:29:37 -0500	[thread overview]
Message-ID: <20140210212931.GA16154@sigill.intra.peff.net> (raw)
In-Reply-To: <20140210191521.GA3112@sigill.intra.peff.net>

We do not run the httpd nor git-daemon tests by default, as
they are rather heavyweight and require network access
(albeit over localhost). However, it would be nice if more
pepole ran them, for two reasons:

  1. We would get more test coverage on more systems.

  2. The point of the test suite is to find regressions. It
     is very easy to change some of the underlying code and
     break the httpd code without realizing you are even
     affecting it. Running the httpd tests helps find these
     problems sooner (ideally before the patches even hit
     the list).

We still want to leave an "out", though, for people who
really do not want to run them. For that reason, the
GIT_TEST_HTTPD and GIT_TEST_GIT_DAEMON variables are now
tri-state booleans (true/false/auto), so you can say
GIT_TEST_HTTPD=false to turn the tests back off.

In addition, we are forgiving of common setup failures
(e.g., you do not have apache installed, or have an old
version) when the tri-state is "auto" (or empty), but report
an error when it is "true". This makes "auto" a sane
default, as we should not cause failures on setups where the
tests cannot run. But it allows people who use "true" to
catch regressions in their system (e.g., they uninstalled
apache, but were expecting their automated test runs to test
git-httpd, and would want to be notified).

Signed-off-by: Jeff King <peff@peff.net>
---

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.

 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.
 
-- 
1.8.5.2.500.g8060133

  parent reply	other threads:[~2014-02-10 21:29 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-06 15:10 [PATCH 0/6] Fix the shallow deepen bug with no-done Nguyễn Thái Ngọc Duy
2014-02-06 15:10 ` [PATCH 1/6] test: rename http fetch and push test files Nguyễn Thái Ngọc Duy
2014-02-06 19:33   ` Jeff King
2014-02-06 15:10 ` [PATCH 2/6] t5538: fix default http port Nguyễn Thái Ngọc Duy
2014-02-06 19:35   ` Jeff King
2014-02-07 23:47     ` Jeff King
2014-02-08  7:36       ` Duy Nguyen
2014-02-10 14:39         ` Jeff King
2014-02-10 18:23           ` Junio C Hamano
2014-02-10 19:15             ` Jeff King
2014-02-10 19:16               ` Jeff King
2014-02-10 21:29               ` Jeff King [this message]
2014-02-11 19:51                 ` [PATCH] tests: turn on network daemon tests by default Junio C Hamano
2014-02-11 20:04                   ` Jeff King
2014-02-11 23:58                     ` Junio C Hamano
2014-02-12 21:47                       ` Jeff King
2014-02-12 22:34                         ` Junio C Hamano
2014-02-13 19:35                           ` Junio C Hamano
2014-02-14  9:58                             ` Jeff King
2014-02-14 16:13                               ` Junio C Hamano
2014-02-12 19:06                     ` Junio C Hamano
2014-02-12 22:12                       ` Jeff King
2014-02-13  1:22                         ` Duy Nguyen
2014-02-13 13:21                           ` [PATCH] t5537: move http tests out to t5539 Nguyễn Thái Ngọc Duy
2014-02-13 20:14                             ` Junio C Hamano
2014-02-06 15:10 ` [PATCH 3/6] pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done' Nguyễn Thái Ngọc Duy
2014-02-06 18:54   ` Junio C Hamano
2014-02-06 15:10 ` [PATCH 4/6] protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt Nguyễn Thái Ngọc Duy
2014-02-06 15:10 ` [PATCH 5/6] protocol-capabilities.txt: document no-done Nguyễn Thái Ngọc Duy
2014-02-06 18:55   ` Junio C Hamano
2014-02-06 19:40   ` Jeff King
2014-02-06 15:10 ` [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap Nguyễn Thái Ngọc Duy
2014-02-06 19:16   ` Junio C Hamano
2014-02-07  0:52     ` Duy Nguyen
2014-02-06 19:35   ` Eric Sunshine
2014-02-06 19:42   ` Jeff King
2014-02-07 18:01   ` Junio C Hamano
2014-02-07 23:39     ` Duy Nguyen
2014-02-10 18:18       ` Junio C Hamano
2014-02-06 19:31 ` [PATCH 0/6] Fix the shallow deepen bug with no-done Junio C Hamano
2014-02-06 19:44   ` Jeff King
2014-02-07  0:47   ` Duy Nguyen
2014-02-07 19:20     ` Jonathan Nieder
2014-02-07 20:03       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140210212931.GA16154@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.