[PATCH 1/3] t/t5400-send-pack.sh: use the canonical test_expect_success syntax

[PATCH 1/3] t/t5400-send-pack.sh: use the canonical test_expect_success syntax

[Ævar Arnfjörð Bjarmason]

Change t/t5400-send-pack.sh to use the usual test_expect_success
syntax, i.e. change:

    test_expect_success \
        'description'

To:

    test_expect_success 'description'

Like the rest of the tests in t/t5400-send-pack.sh do.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5400-send-pack.sh |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index b0b2684..656ab1a 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -76,8 +76,7 @@ test_expect_success 'refuse pushing rewound head without --force' '
 	test "$victim_head" = "$pushed_head"
 '
 
-test_expect_success \
-        'push can be used to delete a ref' '
+test_expect_success 'push can be used to delete a ref' '
 	( cd victim && git branch extra master ) &&
 	git send-pack ./victim :extra master &&
 	( cd victim &&
-- 
1.7.2.3

[PATCH 2/3] t/t5400-send-pack.sh: add test_when_finished commands

[Ævar Arnfjörð Bjarmason]

Change the tests in t/t5400-send-pack.sh to use test_when_finished to
unset git config variables that were set in the tests, e.g.:

    git config --unset $variable_they_set

This doesn't change the test behavior, but makes each test more
self-contained.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5400-send-pack.sh |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 656ab1a..0357610 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -90,7 +90,8 @@ test_expect_success 'refuse deleting push with denyDeletes' '
 	    git config receive.denyDeletes true &&
 	    git branch extra master
 	) &&
-	test_must_fail git send-pack ./victim :extra master
+	test_must_fail git send-pack ./victim :extra master &&
+	test_when_finished "(cd victim && git config --unset receive.denyDeletes)"
 '
 
 test_expect_success 'cannot override denyDeletes with git -c send-pack' '
@@ -101,7 +102,8 @@ test_expect_success 'cannot override denyDeletes with git -c send-pack' '
 		git branch extra master
 	) &&
 	test_must_fail git -c receive.denyDeletes=false \
-					send-pack ./victim :extra master
+					send-pack ./victim :extra master &&
+	test_when_finished "(cd victim && git config --unset receive.denyDeletes)"
 '
 
 test_expect_success 'override denyDeletes with git -c receive-pack' '
@@ -125,7 +127,8 @@ test_expect_success 'denyNonFastforwards trumps --force' '
 	victim_orig=$(cd victim && git rev-parse --verify master) &&
 	test_must_fail git send-pack --force ./victim master^:master &&
 	victim_head=$(cd victim && git rev-parse --verify master) &&
-	test "$victim_orig" = "$victim_head"
+	test "$victim_orig" = "$victim_head" &&
+	test_when_finished "(cd victim && git config --unset receive.denyNonFastforwards)"
 '
 
 test_expect_success 'push --all excludes remote-tracking hierarchy' '
-- 
1.7.2.3

Re: [PATCH 2/3] t/t5400-send-pack.sh: add test_when_finished commands

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 656ab1a..0357610 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -90,7 +90,8 @@ test_expect_success 'refuse deleting push with denyDeletes' '
>  	    git config receive.denyDeletes true &&
>  	    git branch extra master
>  	) &&
> -	test_must_fail git send-pack ./victim :extra master
> +	test_must_fail git send-pack ./victim :extra master &&
> +	test_when_finished "(cd victim && git config --unset receive.denyDeletes)"
>  '

I agree that you are trying to do a good thing, but does this
implementation work in practice?  If somebody breaks send-pack in such a
way that this push succeeds, doesn't the &&-chain will prevent your
"when-finished" hook to be registered to begin with?  IOW, wouldn't you
want to have when-finished as _early_ as possible, before any test that
might fail to help us identify a new breakage?

The same comment applies to other two hunks in the patch.

[PATCH 3/3] t/t5400-send-pack.sh: re-indent subshell invocation

[Ævar Arnfjörð Bjarmason]

Change a subshell invocation in t/t5400-send-pack.sh to use one line
per command in the subshell, that's the usual syntax we use for tests.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5400-send-pack.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 0357610..d4db5ac 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -32,7 +32,11 @@ test_expect_success setup '
 	done &&
 	git update-ref HEAD "$commit" &&
 	git clone ./. victim &&
-	( cd victim && git config receive.denyCurrentBranch warn && git log ) &&
+	(
+	    cd victim &&
+	    git config receive.denyCurrentBranch warn &&
+	    git log
+	) &&
 	git update-ref HEAD "$zero" &&
 	parent=$zero &&
 	i=0 &&
-- 
1.7.2.3