git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Derrick Stolee" <dstolee@microsoft.com>,
	"Brandon Williams" <bwilliams.eng@gmail.com>,
	git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH 2/3] t5703: run all non-httpd-specific tests before sourcing 'lib-httpd.sh'
Date: Thu,  1 Aug 2019 17:53:08 +0200	[thread overview]
Message-ID: <20190801155309.15276-3-szeder.dev@gmail.com> (raw)
In-Reply-To: <20190801155309.15276-1-szeder.dev@gmail.com>

't5703-upload-pack-ref-in-want.sh' sources 'lib-httpd.sh' near the end
to run a couple of httpd-specific tests, but 'lib-httpd.sh' skips all
the rest of the test script if the dependencies for running httpd
tests are not fulfilled.  However, the last six tests in 't5703' are
not httpd-specific, but they are skipped as well when httpd tests
can't be run.

Move these six tests earlier in the test script, before 'lib-httpd.sh'
is sourced, so they will be run even when httpd tests aren't.  Note
that this is not merely a pure code movement, because the setup test
case for the httpd tests needed an additional 'rm -rf
"$LOCAL_PRISTINE"' to clean up a directory left behind by the moved
non-httpd-specific tests.

Also add a comment at the end of this test script to warn against
adding non-httpd-specific tests at the end, in the hope that it will
help prevent similar issues in the future.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/t5703-upload-pack-ref-in-want.sh | 204 +++++++++++++++--------------
 1 file changed, 104 insertions(+), 100 deletions(-)

diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
index de4b6106ef..3a2c143c6d 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -157,106 +157,6 @@ test_expect_success 'want-ref with ref we already have commit for' '
 	check_output
 '
 
-. "$TEST_DIRECTORY"/lib-httpd.sh
-start_httpd
-
-REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo"
-LOCAL_PRISTINE="$(pwd)/local_pristine"
-
-test_expect_success 'setup repos for change-while-negotiating test' '
-	(
-		git init "$REPO" &&
-		cd "$REPO" &&
-		>.git/git-daemon-export-ok &&
-		test_commit m1 &&
-		git tag -d m1 &&
-
-		# Local repo with many commits (so that negotiation will take
-		# more than 1 request/response pair)
-		git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" &&
-		cd "$LOCAL_PRISTINE" &&
-		git checkout -b side &&
-		test_commit_bulk --id=s 33 &&
-
-		# Add novel commits to upstream
-		git checkout master &&
-		cd "$REPO" &&
-		test_commit m2 &&
-		test_commit m3 &&
-		git tag -d m2 m3
-	) &&
-	git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo" &&
-	git -C "$LOCAL_PRISTINE" config protocol.version 2
-'
-
-inconsistency () {
-	# Simulate that the server initially reports $2 as the ref
-	# corresponding to $1, and after that, $1 as the ref corresponding to
-	# $1. This corresponds to the real-life situation where the server's
-	# repository appears to change during negotiation, for example, when
-	# different servers in a load-balancing arrangement serve (stateless)
-	# RPCs during a single negotiation.
-	printf "s/%s/%s/" \
-	       $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
-	       $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
-	       >"$HTTPD_ROOT_PATH/one-time-sed"
-}
-
-test_expect_success 'server is initially ahead - no ref in want' '
-	git -C "$REPO" config uploadpack.allowRefInWant false &&
-	rm -rf local &&
-	cp -r "$LOCAL_PRISTINE" local &&
-	inconsistency master 1234567890123456789012345678901234567890 &&
-	test_must_fail git -C local fetch 2>err &&
-	test_i18ngrep "fatal: remote error: upload-pack: not our ref" err
-'
-
-test_expect_success 'server is initially ahead - ref in want' '
-	git -C "$REPO" config uploadpack.allowRefInWant true &&
-	rm -rf local &&
-	cp -r "$LOCAL_PRISTINE" local &&
-	inconsistency master 1234567890123456789012345678901234567890 &&
-	git -C local fetch &&
-
-	git -C "$REPO" rev-parse --verify master >expected &&
-	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success 'server is initially behind - no ref in want' '
-	git -C "$REPO" config uploadpack.allowRefInWant false &&
-	rm -rf local &&
-	cp -r "$LOCAL_PRISTINE" local &&
-	inconsistency master "master^" &&
-	git -C local fetch &&
-
-	git -C "$REPO" rev-parse --verify "master^" >expected &&
-	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success 'server is initially behind - ref in want' '
-	git -C "$REPO" config uploadpack.allowRefInWant true &&
-	rm -rf local &&
-	cp -r "$LOCAL_PRISTINE" local &&
-	inconsistency master "master^" &&
-	git -C local fetch &&
-
-	git -C "$REPO" rev-parse --verify "master" >expected &&
-	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success 'server loses a ref - ref in want' '
-	git -C "$REPO" config uploadpack.allowRefInWant true &&
-	rm -rf local &&
-	cp -r "$LOCAL_PRISTINE" local &&
-	echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" &&
-	test_must_fail git -C local fetch 2>err &&
-
-	test_i18ngrep "fatal: remote error: unknown ref refs/heads/raster" err
-'
-
 REPO="$(pwd)/repo"
 LOCAL_PRISTINE="$(pwd)/local_pristine"
 
@@ -372,4 +272,108 @@ test_expect_success 'fetching with wildcard that matches multiple refs' '
 	grep "want-ref refs/heads/o/bar" log
 '
 
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo"
+LOCAL_PRISTINE="$(pwd)/local_pristine"
+
+test_expect_success 'setup repos for change-while-negotiating test' '
+	(
+		git init "$REPO" &&
+		cd "$REPO" &&
+		>.git/git-daemon-export-ok &&
+		test_commit m1 &&
+		git tag -d m1 &&
+
+		# Local repo with many commits (so that negotiation will take
+		# more than 1 request/response pair)
+		rm -rf "$LOCAL_PRISTINE" &&
+		git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" &&
+		cd "$LOCAL_PRISTINE" &&
+		git checkout -b side &&
+		test_commit_bulk --id=s 33 &&
+
+		# Add novel commits to upstream
+		git checkout master &&
+		cd "$REPO" &&
+		test_commit m2 &&
+		test_commit m3 &&
+		git tag -d m2 m3
+	) &&
+	git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo" &&
+	git -C "$LOCAL_PRISTINE" config protocol.version 2
+'
+
+inconsistency () {
+	# Simulate that the server initially reports $2 as the ref
+	# corresponding to $1, and after that, $1 as the ref corresponding to
+	# $1. This corresponds to the real-life situation where the server's
+	# repository appears to change during negotiation, for example, when
+	# different servers in a load-balancing arrangement serve (stateless)
+	# RPCs during a single negotiation.
+	printf "s/%s/%s/" \
+	       $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
+	       $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
+	       >"$HTTPD_ROOT_PATH/one-time-sed"
+}
+
+test_expect_success 'server is initially ahead - no ref in want' '
+	git -C "$REPO" config uploadpack.allowRefInWant false &&
+	rm -rf local &&
+	cp -r "$LOCAL_PRISTINE" local &&
+	inconsistency master 1234567890123456789012345678901234567890 &&
+	test_must_fail git -C local fetch 2>err &&
+	test_i18ngrep "fatal: remote error: upload-pack: not our ref" err
+'
+
+test_expect_success 'server is initially ahead - ref in want' '
+	git -C "$REPO" config uploadpack.allowRefInWant true &&
+	rm -rf local &&
+	cp -r "$LOCAL_PRISTINE" local &&
+	inconsistency master 1234567890123456789012345678901234567890 &&
+	git -C local fetch &&
+
+	git -C "$REPO" rev-parse --verify master >expected &&
+	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'server is initially behind - no ref in want' '
+	git -C "$REPO" config uploadpack.allowRefInWant false &&
+	rm -rf local &&
+	cp -r "$LOCAL_PRISTINE" local &&
+	inconsistency master "master^" &&
+	git -C local fetch &&
+
+	git -C "$REPO" rev-parse --verify "master^" >expected &&
+	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'server is initially behind - ref in want' '
+	git -C "$REPO" config uploadpack.allowRefInWant true &&
+	rm -rf local &&
+	cp -r "$LOCAL_PRISTINE" local &&
+	inconsistency master "master^" &&
+	git -C local fetch &&
+
+	git -C "$REPO" rev-parse --verify "master" >expected &&
+	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'server loses a ref - ref in want' '
+	git -C "$REPO" config uploadpack.allowRefInWant true &&
+	rm -rf local &&
+	cp -r "$LOCAL_PRISTINE" local &&
+	echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" &&
+	test_must_fail git -C local fetch 2>err &&
+
+	test_i18ngrep "fatal: remote error: unknown ref refs/heads/raster" err
+'
+
+# DO NOT add non-httpd-specific tests here, because the last part of this
+# test script is only executed when httpd is available and enabled.
+
 test_done
-- 
2.22.0.926.g602b9a0287


  parent reply	other threads:[~2019-08-01 15:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-18 20:25 [PATCH 0/3] fetch: add --[no-]show-forced-updates Derrick Stolee via GitGitGadget
2019-06-18 20:25 ` [PATCH 1/3] fetch: add --[no-]show-forced-updates argument Derrick Stolee via GitGitGadget
2019-07-30 21:29   ` [PATCH] t5510-fetch: fix negated 'test_i18ngrep' invocation SZEDER Gábor
2019-07-30 21:40     ` SZEDER Gábor
2019-07-31 10:35       ` Derrick Stolee
2019-08-01 15:53       ` [PATCH 0/3] tests: run non-httpd-specific tests before sourcing 'lib-httpd.sh' SZEDER Gábor
2019-08-01 15:53         ` [PATCH 1/3] t5510-fetch: run non-httpd-specific test " SZEDER Gábor
2019-08-01 17:51           ` Derrick Stolee
2019-08-01 15:53         ` SZEDER Gábor [this message]
2019-08-01 15:53         ` [PATCH 3/3] tests: warn against appending non-httpd-specific tests at the end SZEDER Gábor
2019-08-01 17:41           ` SZEDER Gábor
2019-08-01 18:18             ` Junio C Hamano
2019-08-02 10:09               ` SZEDER Gábor
2019-08-02 16:37                 ` Junio C Hamano
2019-06-18 20:25 ` [PATCH 2/3] fetch: warn about forced updates in branch listing Derrick Stolee via GitGitGadget
2019-06-18 20:25 ` [PATCH 3/3] pull: add --[no-]show-forced-updates passthrough Derrick Stolee via GitGitGadget

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190801155309.15276-3-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=bwilliams.eng@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH 2/3] t5703: run all non-httpd-specific tests before sourcing '\''lib-httpd.sh'\''' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).