diff mbox series

[v1,2/4] t1301: use test_when_finished for cleanup

Message ID 20221127145130.16155-2-worldhello.net@gmail.com (mailing list archive)
State Accepted
Commit 5d64229ef5a98537bdea20faadfb5cf54ed077f7
Headers show
Series [v1,1/4] t1301: fix wrong template dir for git-init | expand

Commit Message

Jiang Xin Nov. 27, 2022, 2:51 p.m. UTC 
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 t/t1301-shared-repo.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Junio C Hamano Nov. 28, 2022, 4:11 a.m. UTC | #1 
Jiang Xin <worldhello.net@gmail.com> writes:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  t/t1301-shared-repo.sh | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> index 7578e75d77..1225abbb6d 100755
> --- a/t/t1301-shared-repo.sh
> +++ b/t/t1301-shared-repo.sh
> @@ -25,6 +25,7 @@ test_expect_success 'shared = 0400 (faulty permission u-w)' '
>  for u in 002 022
>  do
>  	test_expect_success POSIXPERM "shared=1 does not clear bits preset by umask $u" '
> +		test_when_finished "rm -rf sub" &&
>  		mkdir sub && (
>  			cd sub &&
>  			umask $u &&
> @@ -42,7 +43,6 @@ do
>  			;;
>  		esac
>  	'
> -	rm -rf sub
>  done

This is indeed "we used to clean-up outside the test, but instead
let's use test_when_finished for that".


> @@ -132,6 +132,7 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
>  '
>  
>  test_expect_success POSIXPERM 'forced modes' '
> +	test_when_finished "rm -rf new" &&
>  	mkdir -p templates/hooks &&
>  	echo update-server-info >templates/hooks/post-update &&
>  	chmod +x templates/hooks/post-update &&

But strictly speaking this is different.  We used to leave "new"
after we are done, now we do clean up.  Which is definitely an
improvement in any way ;-)

> @@ -174,6 +175,7 @@ test_expect_success POSIXPERM 'forced modes' '
>  '
>  
>  test_expect_success POSIXPERM 'remote init does not use config from cwd' '
> +	test_when_finished "rm -rf child.git" &&
>  	git config core.sharedrepository 0666 &&
>  	umask 0022 &&
>  	git init --bare child.git &&
> @@ -193,7 +195,7 @@ test_expect_success POSIXPERM 're-init respects core.sharedrepository (local)' '
>  '
>  
>  test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)' '
> -	rm -rf child.git &&
> +	test_when_finished "rm -rf child.git" &&
>  	umask 0022 &&
>  	git init --bare --shared=0666 child.git &&
>  	test_path_is_missing child.git/foo &&
> @@ -204,7 +206,7 @@ test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)'
>  '
>  
>  test_expect_success POSIXPERM 'template can set core.sharedrepository' '
> -	rm -rf child.git &&
> +	test_when_finished "rm -rf child.git" &&
>  	umask 0022 &&
>  	git config core.sharedrepository 0666 &&
>  	cp .git/config templates/config &&

These child.git test repositories used to follow "initialize what we
are going to use to a known state before we use" pattern, which is
not wrong per-se, but now we use "clean up the cruft we make after
we are done" pattern, which may arguably be better, simply because
the test that makes cruft should know what cruft it created better
than whatever comes later that may not know.

OK.
diff mbox series

Patch

diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 7578e75d77..1225abbb6d 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -25,6 +25,7 @@  test_expect_success 'shared = 0400 (faulty permission u-w)' '
 for u in 002 022
 do
 	test_expect_success POSIXPERM "shared=1 does not clear bits preset by umask $u" '
+		test_when_finished "rm -rf sub" &&
 		mkdir sub && (
 			cd sub &&
 			umask $u &&
@@ -42,7 +43,6 @@  do
 			;;
 		esac
 	'
-	rm -rf sub
 done
 
 test_expect_success 'shared=all' '
@@ -132,6 +132,7 @@  test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
 '
 
 test_expect_success POSIXPERM 'forced modes' '
+	test_when_finished "rm -rf new" &&
 	mkdir -p templates/hooks &&
 	echo update-server-info >templates/hooks/post-update &&
 	chmod +x templates/hooks/post-update &&
@@ -174,6 +175,7 @@  test_expect_success POSIXPERM 'forced modes' '
 '
 
 test_expect_success POSIXPERM 'remote init does not use config from cwd' '
+	test_when_finished "rm -rf child.git" &&
 	git config core.sharedrepository 0666 &&
 	umask 0022 &&
 	git init --bare child.git &&
@@ -193,7 +195,7 @@  test_expect_success POSIXPERM 're-init respects core.sharedrepository (local)' '
 '
 
 test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)' '
-	rm -rf child.git &&
+	test_when_finished "rm -rf child.git" &&
 	umask 0022 &&
 	git init --bare --shared=0666 child.git &&
 	test_path_is_missing child.git/foo &&
@@ -204,7 +206,7 @@  test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)'
 '
 
 test_expect_success POSIXPERM 'template can set core.sharedrepository' '
-	rm -rf child.git &&
+	test_when_finished "rm -rf child.git" &&
 	umask 0022 &&
 	git config core.sharedrepository 0666 &&
 	cp .git/config templates/config &&