[v3,4/4] tests: define GIT_TEST_SIDEBAND_ALL

Commit Message

Jonathan Tan Jan. 16, 2019, 7:28 p.m. UTC

Define a GIT_TEST_SIDEBAND_ALL environment variable meant to be used
from tests. When set to true, this overrides uploadpack.allowsidebandall
to true, allowing the entire test suite to be run as if this
configuration is in place for all repositories.

As of this patch, all tests pass whether GIT_TEST_SIDEBAND_ALL is unset
or set to 1.

Signed-off-by: Jonathan Tan <[e-mail]>
Signed-off-by: Junio C Hamano <[e-mail]>
---
 fetch-pack.c             |  3 ++-
 t/README                 |  5 +++++
 t/lib-httpd/apache.conf  |  1 +
 t/t5537-fetch-shallow.sh |  3 ++-
 t/t5701-git-serve.sh     |  2 +-
 t/t5702-protocol-v2.sh   |  4 ++--
 upload-pack.c            | 13 ++++++++-----
 7 files changed, 21 insertions(+), 10 deletions(-)

Comments

Jonathan Nieder Jan. 29, 2019, 11:27 p.m. UTC | #1

Hi,

Jonathan Tan wrote:

> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -78,6 +78,7 @@ PassEnv GNUPGHOME
>  PassEnv ASAN_OPTIONS
>  PassEnv GIT_TRACE
>  PassEnv GIT_CONFIG_NOSYSTEM
> +PassEnv GIT_TEST_SIDEBAND_ALL

This is producing

 [env:warn] [pid 248632] AH01506: PassEnv variable GIT_TEST_SIDEBAND_ALL was undefined

when tests are run with "prove".

Should we set the envvar unconditionally in lib-httpd.sh?

Thanks,
Jonathan

Jeff King Feb. 13, 2019, 6:49 a.m. UTC | #2

On Tue, Jan 29, 2019 at 03:27:32PM -0800, Jonathan Nieder wrote:

> Jonathan Tan wrote:
> 
> > --- a/t/lib-httpd/apache.conf
> > +++ b/t/lib-httpd/apache.conf
> > @@ -78,6 +78,7 @@ PassEnv GNUPGHOME
> >  PassEnv ASAN_OPTIONS
> >  PassEnv GIT_TRACE
> >  PassEnv GIT_CONFIG_NOSYSTEM
> > +PassEnv GIT_TEST_SIDEBAND_ALL
> 
> This is producing
> 
>  [env:warn] [pid 248632] AH01506: PassEnv variable GIT_TEST_SIDEBAND_ALL was undefined
> 
> when tests are run with "prove".
> 
> Should we set the envvar unconditionally in lib-httpd.sh?

Yes, that's the same solution we've had to use for GIT_VALGRIND, etc. I
don't think there's an easier way.

-Peff

Patch

diff --git a/fetch-pack.c b/fetch-pack.c
index 86e9e18901..e98294d918 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1327,7 +1327,8 @@  static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	packet_reader_init(&reader, fd[0], NULL, 0,
 			   PACKET_READ_CHOMP_NEWLINE |
 			   PACKET_READ_DIE_ON_ERR_PACKET);
-	if (server_supports_feature("fetch", "sideband-all", 0)) {
+	if (git_env_bool("GIT_TEST_SIDEBAND_ALL", 1) &&
+	    server_supports_feature("fetch", "sideband-all", 0)) {
 		reader.use_sideband = 1;
 		reader.me = "fetch-pack";
 	}
diff --git a/t/README b/t/README
index 28711cc508..b275c883b8 100644
--- a/t/README
+++ b/t/README
@@ -358,6 +358,11 @@  GIT_TEST_MULTI_PACK_INDEX=<boolean>, when true, forces the multi-pack-
 index to be written after every 'git repack' command, and overrides the
 'core.multiPackIndex' setting to true.
 
+GIT_TEST_SIDEBAND_ALL=<boolean>, when true, overrides the
+'uploadpack.allowSidebandAll' setting to true, and when false, forces
+fetch-pack to not request sideband-all (even if the server advertises
+sideband-all).
+
 Naming Tests
 ------------
 
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 581c010d8f..9a6d368247 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -78,6 +78,7 @@  PassEnv GNUPGHOME
 PassEnv ASAN_OPTIONS
 PassEnv GIT_TRACE
 PassEnv GIT_CONFIG_NOSYSTEM
+PassEnv GIT_TEST_SIDEBAND_ALL
 
 SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0
 
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 6faf17e17a..6caf628efa 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -243,7 +243,8 @@  test_expect_success 'shallow fetches check connectivity before writing shallow f
 	       "$(git -C "$REPO" rev-parse HEAD)" \
 	       "$(git -C "$REPO" rev-parse HEAD^)" \
 	       >"$HTTPD_ROOT_PATH/one-time-sed" &&
-	test_must_fail git -C client fetch --depth=1 "$HTTPD_URL/one_time_sed/repo" \
+	test_must_fail env GIT_TEST_SIDEBAND_ALL=0 git -C client \
+		fetch --depth=1 "$HTTPD_URL/one_time_sed/repo" \
 		master:a_branch &&
 
 	# Ensure that the one-time-sed script was used.
diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index ae79c6bbc0..fe45bf828d 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -14,7 +14,7 @@  test_expect_success 'test capability advertisement' '
 	0000
 	EOF
 
-	git serve --advertise-capabilities >out &&
+	GIT_TEST_SIDEBAND_ALL=0 git serve --advertise-capabilities >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 0f2b09ebb8..b491c62e3e 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -583,8 +583,8 @@  test_expect_success 'when server does not send "ready", expect FLUSH' '
 	test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" git -C http_child \
 		-c protocol.version=2 \
 		fetch "$HTTPD_URL/one_time_sed/http_parent" 2> err &&
-	grep "fetch< acknowledgments" log &&
-	! grep "fetch< ready" log &&
+	grep "fetch< .*acknowledgments" log &&
+	! grep "fetch< .*ready" log &&
 	test_i18ngrep "expected no other sections to be sent after no .ready." err
 '
 
diff --git a/upload-pack.c b/upload-pack.c
index 765b7695d2..0c1feccaab 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1288,7 +1288,9 @@  static void process_args(struct packet_reader *request,
 			continue;
 		}
 
-		if (allow_sideband_all && !strcmp(arg, "sideband-all")) {
+		if ((git_env_bool("GIT_TEST_SIDEBAND_ALL", 0) ||
+		     allow_sideband_all) &&
+		    !strcmp(arg, "sideband-all")) {
 			data->writer.use_sideband = 1;
 			continue;
 		}
@@ -1521,10 +1523,11 @@  int upload_pack_advertise(struct repository *r,
 		    allow_ref_in_want)
 			strbuf_addstr(value, " ref-in-want");
 
-		if (!repo_config_get_bool(the_repository,
-					 "uploadpack.allowsidebandall",
-					 &allow_sideband_all_value) &&
-		    allow_sideband_all_value)
+		if (git_env_bool("GIT_TEST_SIDEBAND_ALL", 0) ||
+		    (!repo_config_get_bool(the_repository,
+					   "uploadpack.allowsidebandall",
+					   &allow_sideband_all_value) &&
+		     allow_sideband_all_value))
 			strbuf_addstr(value, " sideband-all");
 	}