git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 06/13] fetch+push tests: use "test_hook" and "test_when_finished" pattern

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 17, 2022, 10:13 UTC
Message-ID
<patch-v3-06.13-0cf152dfca7-20220317T100820Z-avarab@gmail.com>
In-Reply-To
<cover-v3-00.13-00000000000-20220317T100820Z-avarab@gmail.com>

Change the "t5516-fetch-push.sh" test code to make use of the new "test_hook" helper, and to use "test_when_finished" to have tests clean up their own state, instead of relying on subsequent tests to clean the trash directory.

Before this each test would have been responsible for cleaning up after a preceding test (which may or may not have run, e.g. if --run or "GIT_SKIP_TESTS" was used), now each test will instead clean up after itself.

In order to use both "test_hook" and "test_when_finished" we need to move them out of sub-shells, which requires some refactoring.

While we're at it split up the "push with negotiation" test, now the middle of the test doesn't need to "rm event", and since it delimited two halves that were testing two different things the end-state is easier to read and reason about.

While changing these lines make the minor change from "-fr" to "-rf" as the "rm" argument, some of them used it already, it's more common in the test suite, and it leaves the end-state of the file with more consistency.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5516-fetch-push.sh | 191 +++++++++++++++++++-----------------------
 1 file changed, 88 insertions(+), 103 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 1a20e54adc1..4dfb080433e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -23,13 +23,10 @@ D=$(pwd)
 
 mk_empty () {
 	repo_name="$1"
-	rm -fr "$repo_name" &&
-	mkdir "$repo_name" &&
-	(
-		cd "$repo_name" &&
-		git init &&
-		git config receive.denyCurrentBranch warn
-	)
+	test_when_finished "rm -rf \"$repo_name\"" &&
+	test_path_is_missing "$repo_name" &&
+	git init "$repo_name" &&
+	git -C "$repo_name" config receive.denyCurrentBranch warn
 }
 
 mk_test () {
@@ -58,40 +55,28 @@ mk_test () {
 mk_test_with_hooks() {
 	repo_name=$1
 	mk_test "$@" &&
-	(
-		cd "$repo_name" &&
-		mkdir .git/hooks &&
-		cd .git/hooks &&
-
-		cat >pre-receive <<-'EOF' &&
-		#!/bin/sh
-		cat - >>pre-receive.actual
-		EOF
-
-		cat >update <<-'EOF' &&
-		#!/bin/sh
-		printf "%s %s %s\n" "$@" >>update.actual
-		EOF
-
-		cat >post-receive <<-'EOF' &&
-		#!/bin/sh
-		cat - >>post-receive.actual
-		EOF
-
-		cat >post-update <<-'EOF' &&
-		#!/bin/sh
-		for ref in "$@"
-		do
-			printf "%s\n" "$ref" >>post-update.actual
-		done
-		EOF
-
-		chmod +x pre-receive update post-receive post-update
-	)
+	test_hook -C "$repo_name" pre-receive <<-'EOF' &&
+	cat - >>pre-receive.actual
+	EOF
+
+	test_hook -C "$repo_name" update <<-'EOF' &&
+	printf "%s %s %s\n" "$@" >>update.actual
+	EOF
+
+	test_hook -C "$repo_name" post-receive <<-'EOF' &&
+	cat - >>post-receive.actual
+	EOF
+
+	test_hook -C "$repo_name" post-update <<-'EOF'
+	for ref in "$@"
+	do
+		printf "%s\n" "$ref" >>post-update.actual
+	done
+	EOF
 }
 
 mk_child() {
-	rm -rf "$2" &&
+	test_when_finished "rm -rf \"$2\"" &&
 	git clone "$1" "$2"
 }
 
@@ -196,32 +181,32 @@ grep_wrote () {
 	grep 'write_pack_file/wrote.*"value":"'$1'"' $2
 }
 
-test_expect_success 'push with negotiation' '
-	# Without negotiation
+test_expect_success 'push without negotiation' '
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
 	test_commit -C testrepo unrelated_commit &&
 	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
-	echo now pushing without negotiation &&
+	test_when_finished "rm event" &&
 	GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 push testrepo refs/heads/main:refs/remotes/origin/main &&
-	grep_wrote 5 event && # 2 commits, 2 trees, 1 blob
+	grep_wrote 5 event # 2 commits, 2 trees, 1 blob
+'
 
-	# Same commands, but with negotiation
-	rm event &&
+test_expect_success 'push with negotiation' '
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
 	test_commit -C testrepo unrelated_commit &&
 	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
+	test_when_finished "rm event" &&
 	GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main &&
 	grep_wrote 2 event # 1 commit, 1 tree
 '
 
 test_expect_success 'push with negotiation proceeds anyway even if negotiation fails' '
-	rm event &&
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
 	test_commit -C testrepo unrelated_commit &&
 	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
+	test_when_finished "rm event" &&
 	GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" \
 		git -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
 	grep_wrote 5 event && # 2 commits, 2 trees, 1 blob
@@ -667,7 +652,6 @@ test_expect_success 'push does not update local refs on failure' '
 
 	mk_test testrepo heads/main &&
 	mk_child testrepo child &&
-	mkdir testrepo/.git/hooks &&
 	echo "#!/no/frobnication/today" >testrepo/.git/hooks/pre-receive &&
 	chmod +x testrepo/.git/hooks/pre-receive &&
 	(
@@ -1329,7 +1313,7 @@ done
 
 test_expect_success 'fetch follows tags by default' '
 	mk_test testrepo heads/main &&
-	rm -fr src dst &&
+	test_when_finished "rm -rf src" &&
 	git init src &&
 	(
 		cd src &&
@@ -1339,6 +1323,7 @@ test_expect_success 'fetch follows tags by default' '
 		sed -n "p; s|refs/heads/main$|refs/remotes/origin/main|p" tmp1 |
 		sort -k 3 >../expect
 	) &&
+	test_when_finished "rm -rf dst" &&
 	git init dst &&
 	(
 		cd dst &&
@@ -1364,8 +1349,9 @@ test_expect_success 'peeled advertisements are not considered ref tips' '
 
 test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' '
 	mk_test testrepo heads/main &&
-	rm -fr src dst &&
+	test_when_finished "rm -rf src" &&
 	git init src &&
+	test_when_finished "rm -rf dst" &&
 	git init --bare dst &&
 	(
 		cd src &&
@@ -1388,8 +1374,9 @@ test_expect_success 'pushing a specific ref applies remote.$name.push as refmap'
 
 test_expect_success 'with no remote.$name.push, it is not used as refmap' '
 	mk_test testrepo heads/main &&
-	rm -fr src dst &&
+	test_when_finished "rm -rf src" &&
 	git init src &&
+	test_when_finished "rm -rf dst" &&
 	git init --bare dst &&
 	(
 		cd src &&
@@ -1410,8 +1397,9 @@ test_expect_success 'with no remote.$name.push, it is not used as refmap' '
 
 test_expect_success 'with no remote.$name.push, upstream mapping is used' '
 	mk_test testrepo heads/main &&
-	rm -fr src dst &&
+	test_when_finished "rm -rf src" &&
 	git init src &&
+	test_when_finished "rm -rf dst" &&
 	git init --bare dst &&
 	(
 		cd src &&
@@ -1439,8 +1427,9 @@ test_expect_success 'with no remote.$name.push, upstream mapping is used' '
 
 test_expect_success 'push does not follow tags by default' '
 	mk_test testrepo heads/main &&
-	rm -fr src dst &&
+	test_when_finished "rm -rf src" &&
 	git init src &&
+	test_when_finished "rm -rf dst" &&
 	git init --bare dst &&
 	(
 		cd src &&
@@ -1462,8 +1451,9 @@ test_expect_success 'push does not follow tags by default' '
 
 test_expect_success 'push --follow-tags only pushes relevant tags' '
 	mk_test testrepo heads/main &&
-	rm -fr src dst &&
+	test_when_finished "rm -rf src" &&
 	git init src &&
+	test_when_finished "rm -rf dst" &&
 	git init --bare dst &&
 	(
 		cd src &&
@@ -1501,9 +1491,9 @@ EOF
 '
 
 test_expect_success 'pushing a tag pushes the tagged object' '
-	rm -rf dst.git &&
 	blob=$(echo unreferenced | git hash-object -w --stdin) &&
 	git tag -m foo tag-of-blob $blob &&
+	test_when_finished "rm -rf dst.git" &&
 	git init --bare dst.git &&
 	git push dst.git tag-of-blob &&
 	# the receiving index-pack should have noticed
@@ -1514,7 +1504,7 @@ test_expect_success 'pushing a tag pushes the tagged object' '
 '
 
 test_expect_success 'push into bare respects core.logallrefupdates' '
-	rm -rf dst.git &&
+	test_when_finished "rm -rf dst.git" &&
 	git init --bare dst.git &&
 	git -C dst.git config core.logallrefupdates true &&
 
@@ -1532,7 +1522,7 @@ test_expect_success 'push into bare respects core.logallrefupdates' '
 '
 
 test_expect_success 'fetch into bare respects core.logallrefupdates' '
-	rm -rf dst.git &&
+	test_when_finished "rm -rf dst.git" &&
 	git init --bare dst.git &&
 	(
 		cd dst.git &&
@@ -1553,6 +1543,7 @@ test_expect_success 'fetch into bare respects core.logallrefupdates' '
 '
 
 test_expect_success 'receive.denyCurrentBranch = updateInstead' '
+	mk_empty testrepo &&
 	git push testrepo main &&
 	(
 		cd testrepo &&
@@ -1655,7 +1646,7 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' '
 	) &&
 
 	# (5) push into void
-	rm -fr void &&
+	test_when_finished "rm -rf void" &&
 	git init void &&
 	(
 		cd void &&
@@ -1677,26 +1668,23 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' '
 '
 
 test_expect_success 'updateInstead with push-to-checkout hook' '
-	rm -fr testrepo &&
+	test_when_finished "rm -rf testrepo" &&
 	git init testrepo &&
-	(
-		cd testrepo &&
-		git pull .. main &&
-		git reset --hard HEAD^^ &&
-		git tag initial &&
-		git config receive.denyCurrentBranch updateInstead &&
-		write_script .git/hooks/push-to-checkout <<-\EOF
-		echo >&2 updating from $(git rev-parse HEAD)
-		echo >&2 updating to "$1"
-
-		git update-index -q --refresh &&
-		git read-tree -u -m HEAD "$1" || {
-			status=$?
-			echo >&2 read-tree failed
-			exit $status
-		}
-		EOF
-	) &&
+	git -C testrepo pull .. main &&
+	git -C testrepo reset --hard HEAD^^ &&
+	git -C testrepo tag initial &&
+	git -C testrepo config receive.denyCurrentBranch updateInstead &&
+	test_hook -C testrepo push-to-checkout <<-\EOF &&
+	echo >&2 updating from $(git rev-parse HEAD)
+	echo >&2 updating to "$1"
+
+	git update-index -q --refresh &&
+	git read-tree -u -m HEAD "$1" || {
+		status=$?
+		echo >&2 read-tree failed
+		exit $status
+	}
+	EOF
 
 	# Try pushing into a pristine
 	git push testrepo main &&
@@ -1739,35 +1727,32 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
 	) &&
 
 	# push into void
-	rm -fr void &&
+	test_when_finished "rm -rf void" &&
 	git init void &&
-	(
-		cd void &&
-		git config receive.denyCurrentBranch updateInstead &&
-		write_script .git/hooks/push-to-checkout <<-\EOF
-		if git rev-parse --quiet --verify HEAD
-		then
-			has_head=yes
-			echo >&2 updating from $(git rev-parse HEAD)
-		else
-			has_head=no
-			echo >&2 pushing into void
-		fi
-		echo >&2 updating to "$1"
-
-		git update-index -q --refresh &&
-		case "$has_head" in
-		yes)
-			git read-tree -u -m HEAD "$1" ;;
-		no)
-			git read-tree -u -m "$1" ;;
-		esac || {
-			status=$?
-			echo >&2 read-tree failed
-			exit $status
-		}
-		EOF
-	) &&
+	git -C void config receive.denyCurrentBranch updateInstead &&
+	test_hook -C void push-to-checkout <<-\EOF &&
+	if git rev-parse --quiet --verify HEAD
+	then
+		has_head=yes
+		echo >&2 updating from $(git rev-parse HEAD)
+	else
+		has_head=no
+		echo >&2 pushing into void
+	fi
+	echo >&2 updating to "$1"
+
+	git update-index -q --refresh &&
+	case "$has_head" in
+	yes)
+		git read-tree -u -m HEAD "$1" ;;
+	no)
+		git read-tree -u -m "$1" ;;
+	esac || {
+		status=$?
+		echo >&2 read-tree failed
+		exit $status
+	}
+	EOF
 
 	git push void main &&
 	(

-- 
2.35.1.1384.g7d2906948a1
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 41 of 51 in “tests: add and use a "test_hook" wrapper + hook fixes”
  1. 00/10 tests: add and use a "test_hook" wrapper + hook fixesÆvar Arnfjörð Bjarmason, Mar 2, 2022
  2. 02/10 t5540: don't rely on "hook/post-update.sample"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  3. Junio C HamanoMar 2, 2022
  4. 01/10 hook tests: turn exit code assertions into a loopÆvar Arnfjörð Bjarmason, Mar 2, 2022
  5. Junio C HamanoMar 2, 2022
  6. 03/10 tests: assume the hooks are disabled by defaultÆvar Arnfjörð Bjarmason, Mar 2, 2022
  7. Junio C HamanoMar 2, 2022
  8. 04/10 bugreport tests: tighten up "git bugreport -s hooks" testÆvar Arnfjörð Bjarmason, Mar 2, 2022
  9. Junio C HamanoMar 2, 2022
  10. 05/10 tests: indent and add hook setup to "test_expect_success"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  11. Junio C HamanoMar 2, 2022
  12. 06/10 hook tests: get rid of unnecessary sub-shellsÆvar Arnfjörð Bjarmason, Mar 2, 2022
  13. Junio C HamanoMar 2, 2022
  14. 07/10 fetch+push tests: have tests clean up their own messÆvar Arnfjörð Bjarmason, Mar 2, 2022
  15. Junio C HamanoMar 2, 2022
  16. 10/10 tests: change "cat && chmod +x" to use "test_hook"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  17. Eric SunshineMar 3, 2022
  18. 08/10 test-lib-functions: add and use a "test_hook" wrapperÆvar Arnfjörð Bjarmason, Mar 2, 2022
  19. Junio C HamanoMar 2, 2022
  20. 09/10 tests: change "mkdir -p && write_script" to use "test_hook"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  21. Junio C HamanoMar 2, 2022
  22. Junio C HamanoMar 2, 2022
  23. 00/10 tests: add and use a "test_hook" wrapper + hook fixesÆvar Arnfjörð Bjarmason, Mar 7, 2022
  24. 01/10 hook tests: turn exit code assertions into a loopÆvar Arnfjörð Bjarmason, Mar 7, 2022
  25. 02/10 t5540: don't rely on "hook/post-update.sample"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  26. 03/10 tests: assume the hooks are disabled by defaultÆvar Arnfjörð Bjarmason, Mar 7, 2022
  27. 04/10 bugreport tests: tighten up "git bugreport -s hooks" testÆvar Arnfjörð Bjarmason, Mar 7, 2022
  28. 05/10 tests: indent and add hook setup to "test_expect_success"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  29. 06/10 hook tests: get rid of unnecessary sub-shellsÆvar Arnfjörð Bjarmason, Mar 7, 2022
  30. 08/10 test-lib-functions: add and use a "test_hook" wrapperÆvar Arnfjörð Bjarmason, Mar 7, 2022
  31. 07/10 fetch+push tests: have tests clean up their own messÆvar Arnfjörð Bjarmason, Mar 7, 2022
  32. 10/10 tests: change "cat && chmod +x" to use "test_hook"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  33. 09/10 tests: change "mkdir -p && write_script" to use "test_hook"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  34. Junio C HamanoMar 7, 2022
  35. 00/13 tests: add and use a "test_hook" wrapper + hook fixesÆvar Arnfjörð Bjarmason, Mar 17, 2022
  36. 01/13 test-lib-functions: add and use a "test_hook" wrapperÆvar Arnfjörð Bjarmason, Mar 17, 2022
  37. 03/13 http tests: don't rely on "hook/post-update.sample"Ævar Arnfjörð Bjarmason, Mar 17, 2022
  38. 02/13 hook tests: turn exit code assertions into a loopÆvar Arnfjörð Bjarmason, Mar 17, 2022
  39. 04/13 tests: assume the hooks are disabled by defaultÆvar Arnfjörð Bjarmason, Mar 17, 2022
  40. 05/13 bugreport tests: tighten up "git bugreport -s hooks" testÆvar Arnfjörð Bjarmason, Mar 17, 2022
  41. 06/13 fetch+push tests: use "test_hook" and "test_when_finished" patternÆvar Arnfjörð Bjarmason, Mar 17, 2022
  42. 08/13 tests: change "cat && chmod +x" to use "test_hook"Ævar Arnfjörð Bjarmason, Mar 17, 2022
  43. 10/13 tests: use "test_hook" for misc "mkdir -p" and "chmod" casesÆvar Arnfjörð Bjarmason, Mar 17, 2022
  44. 07/13 gc + p4 tests: use "test_hook", remove sub-shellsÆvar Arnfjörð Bjarmason, Mar 17, 2022
  45. 09/13 tests: change "mkdir -p && write_script" to use "test_hook"Ævar Arnfjörð Bjarmason, Mar 17, 2022
  46. 11/13 tests: extend "test_hook" for "rm" and "chmod -x", convert "$HOOK"Ævar Arnfjörð Bjarmason, Mar 17, 2022
  47. 13/13 http tests: use "test_hook" for "smart" and "dumb" http testsÆvar Arnfjörð Bjarmason, Mar 17, 2022
  48. 12/13 proc-receive hook tests: use "test_hook" instead of "write_script"Ævar Arnfjörð Bjarmason, Mar 17, 2022
  49. Junio C HamanoMar 17, 2022
  50. Ævar Arnfjörð BjarmasonMar 17, 2022
  51. Junio C HamanoMar 17, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.