Re: [PATCH v4 2/8] t5520: test no merge candidates cases

[Johannes Schindelin <johannes.schindelin@gmx.de>]

Hi Paul,

On 2015-05-18 15:32, Paul Tan wrote:

> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 5e4db67..4a2c0a1 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -109,6 +109,61 @@ test_expect_success 'the default remote . should
> not break explicit pull' '
>  	test "$(cat file)" = modified
>  '
>  
> +test_expect_success 'fail if wildcard spec does not match any refs' '
> +	git checkout -b test copy^ &&
> +	test_when_finished "git checkout -f copy && git branch -D test" &&

When I read this line, I immediately asked myself whether the branch would be deleted even if the test case failed. I then tested this theory by editing the first test case ("setup") like this:

-- snip --
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 227d293..3adc702 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -15,9 +15,12 @@ test_expect_success setup '
 
 	echo file >file &&
 	git add file &&
+test_when_finished "rm file" &&
+false &&
 	git commit -a -m original
 
 '
+exit 1
 
 test_expect_success 'pulling into void' '
 	mkdir cloned &&
-- snap --

and indeed, the file "file" was gone, even if the test case failed. I therefore believe that this "test_when_finished" cleanup might make debugging substantially harder. Maybe we can drop these lines from this patch?

Ciao,
Dscho