[PATCH 0/2] Fix a couple of minor things in pull test

[PATCH 0/2] Fix a couple of minor things in pull test

[Ramkumar Ramachandra]

I just happened to open the file when searching for the right file to
put my autostash tests.

Ramkumar Ramachandra (2):
  t5520 (pull): update test description
  t5520 (pull): use test_config where appropriate

 t/t5520-pull.sh | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

-- 
1.8.2.141.gad203c2.dirty

[PATCH 1/2] t5520 (pull): update test description

[Ramkumar Ramachandra]

d09e79c (git-pull: allow pulling into an empty repository, 2006-11-16)
created the file with the description.  At the time, there were only
tests to check pulling into an empty repository.  The description was
never updated even after more tests, unrelated to the original, were
added.  Fix this now.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5520-pull.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 35304b4..e5adee8 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='pulling into void'
+test_description='pull basic functionality'
 
 . ./test-lib.sh
 
-- 
1.8.2.141.gad203c2.dirty

[PATCH 2/2] t5520 (pull): use test_config where appropriate

[Ramkumar Ramachandra]

Configuration from test_config does not last beyond the end of the
current test assertion, making each test easier to think about in
isolation.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5520-pull.sh | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index e5adee8..0fe935b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -60,8 +60,8 @@ test_expect_success 'pulling into void does not overwrite untracked files' '
 test_expect_success 'test . as a remote' '
 
 	git branch copy master &&
-	git config branch.copy.remote . &&
-	git config branch.copy.merge refs/heads/master &&
+	test_config branch.copy.remote . &&
+	test_config branch.copy.merge refs/heads/master &&
 	echo updated >file &&
 	git commit -a -m updated &&
 	git checkout copy &&
@@ -96,8 +96,7 @@ test_expect_success '--rebase' '
 '
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
-	git config --bool pull.rebase true &&
-	test_when_finished "git config --unset pull.rebase" &&
+	test_config pull.rebase true &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' '
 
 test_expect_success 'branch.to-rebase.rebase' '
 	git reset --hard before-rebase &&
-	git config --bool branch.to-rebase.rebase true &&
-	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	test_config branch.to-rebase.rebase true &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' '
 
 test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 	git reset --hard before-rebase &&
-	git config --bool pull.rebase true &&
-	test_when_finished "git config --unset pull.rebase" &&
-	git config --bool branch.to-rebase.rebase false &&
-	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	test_config pull.rebase true &&
+	test_config branch.to-rebase.rebase false &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -171,9 +167,9 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
 	git update-ref refs/remotes/me/copy copy^ &&
 	COPY=$(git rev-parse --verify me/copy) &&
 	git rebase --onto $COPY copy &&
-	git config branch.to-rebase.remote me &&
-	git config branch.to-rebase.merge refs/heads/copy &&
-	git config branch.to-rebase.rebase true &&
+	test_config branch.to-rebase.remote me &&
+	test_config branch.to-rebase.merge refs/heads/copy &&
+	test_config branch.to-rebase.rebase true &&
 	echo dirty >> file &&
 	git add file &&
 	test_must_fail git pull &&
-- 
1.8.2.141.gad203c2.dirty

Re: [PATCH 2/2] t5520 (pull): use test_config where appropriate

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Configuration from test_config does not last beyond the end of the
> current test assertion, making each test easier to think about in
> isolation.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t5520-pull.sh | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index e5adee8..0fe935b 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -60,8 +60,8 @@ test_expect_success 'pulling into void does not overwrite untracked files' '
>  test_expect_success 'test . as a remote' '
>  
>  	git branch copy master &&
> -	git config branch.copy.remote . &&
> -	git config branch.copy.merge refs/heads/master &&
> +	test_config branch.copy.remote . &&
> +	test_config branch.copy.merge refs/heads/master &&
>  	echo updated >file &&
>  	git commit -a -m updated &&
>  	git checkout copy &&

I am not sure if this makes sense.  The "copy" branch this test
piece creates is used throughout the remainder of the test, and
these configuration variables establish a known default for cases
the later test checks when these various forms of "git pull" command
omits "from where" and "which branch".  It feels actively wrong to
discard that information after this test piece is done.

> @@ -96,8 +96,7 @@ test_expect_success '--rebase' '
>  '
>  test_expect_success 'pull.rebase' '
>  	git reset --hard before-rebase &&
> -	git config --bool pull.rebase true &&
> -	test_when_finished "git config --unset pull.rebase" &&
> +	test_config pull.rebase true &&
>  	git pull . copy &&
>  	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
>  	test new = $(git show HEAD:file2)
> @@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' '
>  
>  test_expect_success 'branch.to-rebase.rebase' '
>  	git reset --hard before-rebase &&
> -	git config --bool branch.to-rebase.rebase true &&
> -	test_when_finished "git config --unset branch.to-rebase.rebase" &&
> +	test_config branch.to-rebase.rebase true &&
>  	git pull . copy &&
>  	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
>  	test new = $(git show HEAD:file2)
> @@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' '
>  
>  test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
>  	git reset --hard before-rebase &&
> -	git config --bool pull.rebase true &&
> -	test_when_finished "git config --unset pull.rebase" &&
> -	git config --bool branch.to-rebase.rebase false &&
> -	test_when_finished "git config --unset branch.to-rebase.rebase" &&
> +	test_config pull.rebase true &&
> +	test_config branch.to-rebase.rebase false &&
>  	git pull . copy &&
>  	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
>  	test new = $(git show HEAD:file2)
> @@ -171,9 +167,9 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
>  	git update-ref refs/remotes/me/copy copy^ &&
>  	COPY=$(git rev-parse --verify me/copy) &&
>  	git rebase --onto $COPY copy &&
> -	git config branch.to-rebase.remote me &&
> -	git config branch.to-rebase.merge refs/heads/copy &&
> -	git config branch.to-rebase.rebase true &&
> +	test_config branch.to-rebase.remote me &&
> +	test_config branch.to-rebase.merge refs/heads/copy &&
> +	test_config branch.to-rebase.rebase true &&
>  	echo dirty >> file &&
>  	git add file &&
>  	test_must_fail git pull &&

[PATCH v2 2/2] t5520 (pull): use test_config where appropriate

[Ramkumar Ramachandra]

Configuration from test_config does not last beyond the end of the
current test assertion, making each test easier to think about in
isolation.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Removed first hunk, as per Junio's comment.

 t/t5520-pull.sh | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index e5adee8..87777ea 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -96,8 +96,7 @@ test_expect_success '--rebase' '
 '
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
-	git config --bool pull.rebase true &&
-	test_when_finished "git config --unset pull.rebase" &&
+	test_config pull.rebase true &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' '
 
 test_expect_success 'branch.to-rebase.rebase' '
 	git reset --hard before-rebase &&
-	git config --bool branch.to-rebase.rebase true &&
-	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	test_config branch.to-rebase.rebase true &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' '
 
 test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 	git reset --hard before-rebase &&
-	git config --bool pull.rebase true &&
-	test_when_finished "git config --unset pull.rebase" &&
-	git config --bool branch.to-rebase.rebase false &&
-	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	test_config pull.rebase true &&
+	test_config branch.to-rebase.rebase false &&
 	git pull . copy &&
 	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
@@ -171,9 +167,9 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
 	git update-ref refs/remotes/me/copy copy^ &&
 	COPY=$(git rev-parse --verify me/copy) &&
 	git rebase --onto $COPY copy &&
-	git config branch.to-rebase.remote me &&
-	git config branch.to-rebase.merge refs/heads/copy &&
-	git config branch.to-rebase.rebase true &&
+	test_config branch.to-rebase.remote me &&
+	test_config branch.to-rebase.merge refs/heads/copy &&
+	test_config branch.to-rebase.rebase true &&
 	echo dirty >> file &&
 	git add file &&
 	test_must_fail git pull &&
-- 
1.8.2.141.g07926c6

Re: [PATCH v2 2/2] t5520 (pull): use test_config where appropriate

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Configuration from test_config does not last beyond the end of the
> current test assertion, making each test easier to think about in
> isolation.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Removed first hunk, as per Junio's comment.

Thanks, but doesn't yd/use-test-config-unconfig topic already
address this?

Re: [PATCH v2 2/2] t5520 (pull): use test_config where appropriate

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Thanks, but doesn't yd/use-test-config-unconfig topic already
> address this?

Just saw it.  9d6aa64 is identical to my patch, but misses the fourth hunk.

Re: [PATCH v2 2/2] t5520 (pull): use test_config where appropriate

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Configuration from test_config does not last beyond the end of the
>> current test assertion, making each test easier to think about in
>> isolation.
>>
>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>> ---
>>  Removed first hunk, as per Junio's comment.
>
> Thanks, but doesn't yd/use-test-config-unconfig topic already
> address this?

The last hunk from your version was missing from 9d6aa64dc32b
(t5520: use test_config to set/unset git config variables,
2013-03-24); I'll pick that part and apply as a follow up to the
series.

No need to resend; thanks.