Re: [PATCH 5/5] t1400: use test_when_finished for cleanup

[Jeff King]

On Mon, Mar 20, 2017 at 08:56:16PM -0400, Kyle Meyer wrote:

> Move cleanup lines that occur after test blocks into
> test_when_finished calls within the test bodies.  Don't move cleanup
> lines that seem to be related to mutiple tests rather than a single
> test.

Sounds logical. The patch looks good to me (though I'll admit my eyes
may have glazed a little in the middle).

-Peff

[PATCH 5/5] t1400: use test_when_finished for cleanup

[Kyle Meyer]

Move cleanup lines that occur after test blocks into
test_when_finished calls within the test bodies.  Don't move cleanup
lines that seem to be related to mutiple tests rather than a single
test.

Signed-off-by: Kyle Meyer 
---
 t/t1400-update-ref.sh | 81 ++-
 1 file changed, 41 insertions(+), 40 deletions(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 9e7e0227e..a23cd7b57 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -48,24 +48,24 @@ test_expect_success "fail to delete $m with stale ref" '
test $B = "$(cat .git/$m)"
 '
 test_expect_success "delete $m" '
+   test_when_finished "rm -f .git/$m" &&
git update-ref -d $m $B &&
test_path_is_missing .git/$m
 '
-rm -f .git/$m
 
-test_expect_success "delete $m without oldvalue verification" "
+test_expect_success "delete $m without oldvalue verification" '
+   test_when_finished "rm -f .git/$m" &&
git update-ref $m $A &&
-   test $A = \$(cat .git/$m) &&
+   test $A = $(cat .git/$m) &&
git update-ref -d $m &&
test_path_is_missing .git/$m
-"
-rm -f .git/$m
+'
 
-test_expect_success \
-   "fail to create $n" \
-   "touch .git/$n_dir &&
-test_must_fail git update-ref $n $A"
-rm -f .git/$n_dir
+test_expect_success "fail to create $n" '
+   test_when_finished "rm -f .git/$n_dir" &&
+   touch .git/$n_dir &&
+   test_must_fail git update-ref $n $A
+'
 
 test_expect_success \
"create $m (by HEAD)" \
@@ -80,28 +80,28 @@ test_expect_success "fail to delete $m (by HEAD) with stale 
ref" '
test $B = $(cat .git/$m)
 '
 test_expect_success "delete $m (by HEAD)" '
+   test_when_finished "rm -f .git/$m" &&
git update-ref -d HEAD $B &&
test_path_is_missing .git/$m
 '
-rm -f .git/$m
 
 test_expect_success "deleting current branch adds message to HEAD's log" '
+   test_when_finished "rm -f .git/$m" &&
git update-ref $m $A &&
git symbolic-ref HEAD $m &&
git update-ref -m delete-$m -d $m &&
test_path_is_missing .git/$m &&
grep "delete-$m$" .git/logs/HEAD
 '
-rm -f .git/$m
 
 test_expect_success "deleting by HEAD adds message to HEAD's log" '
+   test_when_finished "rm -f .git/$m" &&
git update-ref $m $A &&
git symbolic-ref HEAD $m &&
git update-ref -m delete-by-head -d HEAD &&
test_path_is_missing .git/$m &&
grep "delete-by-head$" .git/logs/HEAD
 '
-rm -f .git/$m
 
 test_expect_success 'update-ref does not create reflogs by default' '
test_when_finished "git update-ref -d $outside" &&
@@ -188,20 +188,21 @@ test_expect_success \
"git update-ref HEAD $B $A &&
 test $B"' = $(cat .git/'"$m"')'
 test_expect_success "delete $m (by HEAD) should remove both packed and loose 
$m" '
+   test_when_finished "rm -f .git/$m" &&
git update-ref -d HEAD $B &&
! grep "$m" .git/packed-refs &&
test_path_is_missing .git/$m
 '
-rm -f .git/$m
 
 cp -f .git/HEAD .git/HEAD.orig
 test_expect_success "delete symref without dereference" '
+   test_when_finished "cp -f .git/HEAD.orig .git/HEAD" &&
git update-ref --no-deref -d HEAD &&
test_path_is_missing .git/HEAD
 '
-cp -f .git/HEAD.orig .git/HEAD
 
 test_expect_success "delete symref without dereference when the referred ref 
is packed" '
+   test_when_finished "cp -f .git/HEAD.orig .git/HEAD" &&
echo foo >foo.c &&
git add foo.c &&
git commit -m foo &&
@@ -209,7 +210,7 @@ test_expect_success "delete symref without dereference when 
the referred ref is
git update-ref --no-deref -d HEAD &&
test_path_is_missing .git/HEAD
 '
-cp -f .git/HEAD.orig .git/HEAD
+
 git update-ref -d $m
 
 test_expect_success 'update-ref -d is not confused by self-reference' '
@@ -241,10 +242,10 @@ test_expect_success 'update-ref --no-deref -d can delete 
reference to bad ref' '
 test_expect_success '(not) create HEAD with old sha1' "
test_must_fail git update-ref HEAD $A $B
 "
-test_expect_success "(not) prior created .git/$m" "
+test_expect_success "(not) prior created .git/$m" '
+   test_when_finished "rm -f .git/$m" &&
test_path_is_missing .git/$m
-"
-rm -f .git/$m
+'
 
 test_expect_success \
"create HEAD" \
@@ -252,10 +253,10 @@ test_expect_success \
 test_expect_success '(not) change HEAD with wrong SHA1' "
test_must_fail git update-ref HEAD $B $Z
 "
-test_expect_success "(not) changed .git/$m" "
-   ! test $B"' = $(cat .git/'"$m"')
+test_expect_success "(not) changed .git/$m" '
+   test_when_finished "rm -f .git/$m" &&
+   ! test $B = $(cat .git/$m)
 '
-rm -f .git/$m
 
 rm -f .git/logs/refs/heads/master
 test_expect_success \
@@ -309,10 +310,10 @@ $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 
1117150200 + Initial Creati
 $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +  Switch