threads / patch / 26301

patch, 3 partst/t5400-send-pack.sh: use the canonical test_expect_success syntax

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

## tl;dr

4 messages between Jan 18, 2011 and Jan 18, 2011. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Ævar Arnfjörð Bjarmason· Jan 18, 2011, 21:41 UTC · lore

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(-)
Show changes to t/t5400-send-pack.sh +1 −2
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
Ævar Arnfjörð Bjarmason· Jan 18, 2011, 21:41 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to t/t5400-send-pack.sh +6 −3
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
Junio C Hamano· Jan 18, 2011, 23:10 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 12 quoted lines
> 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.
Ævar Arnfjörð Bjarmason· Jan 18, 2011, 21:41 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to t/t5400-send-pack.sh +5 −1
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

← back to recent threads