diff mbox series

[06/10] hook tests: get rid of unnecessary sub-shells

Message ID patch-06.10-d86ee06b46e-20220302T131859Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series tests: add and use a "test_hook" wrapper + hook fixes | expand

Commit Message

Ævar Arnfjörð Bjarmason March 2, 2022, 1:22 p.m. UTC
Refactor the repository setup code for tests that test hooks the use
of sub-shells when setting up the test repository and hooks.

A subsequent commit will change the hook setup to use a helper that
makes use of "test_when_finished", which cannot be used in
sub-shells. Let's change that setup code, and the adjacent and similar
"mk_empty" code.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5516-fetch-push.sh   | 150 ++++++++++++++++++----------------------
 t/t6500-gc.sh           |  18 ++---
 t/t9800-git-p4-basic.sh |  24 ++++---
 3 files changed, 94 insertions(+), 98 deletions(-)

Comments

Junio C Hamano March 2, 2022, 9:31 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Refactor the repository setup code for tests that test hooks the use
> of sub-shells when setting up the test repository and hooks.
>
> A subsequent commit will change the hook setup to use a helper that
> makes use of "test_when_finished", which cannot be used in
> sub-shells. Let's change that setup code, and the adjacent and similar
> "mk_empty" code.

Makes sense.

> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 1a20e54adc1..e4bb7581568 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -24,12 +24,8 @@ D=$(pwd)
>  mk_empty () {
>  	repo_name="$1"
>  	rm -fr "$repo_name" &&
> -	mkdir "$repo_name" &&
> -	(
> -		cd "$repo_name" &&
> -		git init &&
> -		git config receive.denyCurrentBranch warn
> -	)
> +	git init "$repo_name" &&
> +	git -C "$repo_name" config receive.denyCurrentBranch warn
>  }

I thought of this exact change when I saw a change to this part in
this series in an earlier step.  Excellent.
diff mbox series

Patch

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 1a20e54adc1..e4bb7581568 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -24,12 +24,8 @@  D=$(pwd)
 mk_empty () {
 	repo_name="$1"
 	rm -fr "$repo_name" &&
-	mkdir "$repo_name" &&
-	(
-		cd "$repo_name" &&
-		git init &&
-		git config receive.denyCurrentBranch warn
-	)
+	git init "$repo_name" &&
+	git -C "$repo_name" config receive.denyCurrentBranch warn
 }
 
 mk_test () {
@@ -58,36 +54,35 @@  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
-	)
+	cat >"$repo_name"/.git/hooks/pre-receive <<-'EOF' &&
+	#!/bin/sh
+	cat - >>pre-receive.actual
+	EOF
+
+	cat >"$repo_name"/.git/hooks/update <<-'EOF' &&
+	#!/bin/sh
+	printf "%s %s %s\n" "$@" >>update.actual
+	EOF
+
+	cat >"$repo_name"/.git/hooks/post-receive <<-'EOF' &&
+	#!/bin/sh
+	cat - >>post-receive.actual
+	EOF
+
+	cat >"$repo_name"/.git/hooks/post-update <<-'EOF' &&
+	#!/bin/sh
+	for ref in "$@"
+	do
+		printf "%s\n" "$ref" >>post-update.actual
+	done
+	EOF
+
+	chmod +x \
+	      "$repo_name"/.git/hooks/pre-receive \
+	      "$repo_name"/.git/hooks/update \
+	      "$repo_name"/.git/hooks/post-receive \
+	      "$repo_name"/.git/hooks/post-update
 }
 
 mk_child() {
@@ -667,7 +662,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 &&
 	(
@@ -1679,24 +1673,21 @@  test_expect_success 'receive.denyCurrentBranch = updateInstead' '
 test_expect_success 'updateInstead with push-to-checkout hook' '
 	rm -fr 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 &&
+	write_script testrepo/.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
 
 	# Try pushing into a pristine
 	git push testrepo main &&
@@ -1741,33 +1732,30 @@  test_expect_success 'updateInstead with push-to-checkout hook' '
 	# push into void
 	rm -fr 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 &&
+	write_script void/.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 push void main &&
 	(
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index c2021267f2c..a6b0db22867 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -101,12 +101,12 @@  test_expect_success 'pre-auto-gc hook can stop auto gc' '
 	EOF
 
 	git init pre-auto-gc-hook &&
+	write_script "pre-auto-gc-hook/.git/hooks/pre-auto-gc" <<-\EOF &&
+	echo >&2 no gc for you &&
+	exit 1
+	EOF
 	(
 		cd pre-auto-gc-hook &&
-		write_script ".git/hooks/pre-auto-gc" <<-\EOF &&
-		echo >&2 no gc for you &&
-		exit 1
-		EOF
 
 		git config gc.auto 3 &&
 		git config gc.autoDetach false &&
@@ -128,12 +128,14 @@  test_expect_success 'pre-auto-gc hook can stop auto gc' '
 	See "git help gc" for manual housekeeping.
 	EOF
 
+	write_script "pre-auto-gc-hook/.git/hooks/pre-auto-gc" <<-\EOF &&
+	echo >&2 will gc for you &&
+	exit 0
+	EOF
+
 	(
 		cd pre-auto-gc-hook &&
-		write_script ".git/hooks/pre-auto-gc" <<-\EOF &&
-		echo >&2 will gc for you &&
-		exit 0
-		EOF
+
 		git gc --auto >../out.actual 2>../err.actual
 	) &&
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 806005a793a..3c1534c94d6 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -277,16 +277,22 @@  test_expect_success 'run hook p4-pre-submit before submit' '
 		git commit -m "add hello.txt" &&
 		git config git-p4.skipSubmitEdit true &&
 		git p4 submit --dry-run >out &&
-		grep "Would apply" out &&
-		mkdir -p .git/hooks &&
-		write_script .git/hooks/p4-pre-submit <<-\EOF &&
-		exit 0
-		EOF
+		grep "Would apply" out
+	) &&
+	mkdir -p "$git"/.git/hooks &&
+	write_script "$git"/.git/hooks/p4-pre-submit <<-\EOF &&
+	exit 0
+	EOF
+	(
+		cd "$git" &&
 		git p4 submit --dry-run >out &&
-		grep "Would apply" out &&
-		write_script .git/hooks/p4-pre-submit <<-\EOF &&
-		exit 1
-		EOF
+		grep "Would apply" out
+	) &&
+	write_script "$git"/.git/hooks/p4-pre-submit <<-\EOF &&
+	exit 1
+	EOF
+	(
+		cd "$git" &&
 		test_must_fail git p4 submit --dry-run >errs 2>&1 &&
 		! grep "Would apply" errs
 	)