threads / patch / 33256

patch, 2 partsFix a couple of minor things in pull test

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

## tl;dr

8 messages between Mar 22, 2013 and Mar 28, 2013. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Ramkumar Ramachandra· Mar 22, 2013, 12:31 UTC · lore

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
Ramkumar Ramachandra· Mar 22, 2013, 12:31 UTC · re: Ramkumar Ramachandra · lore

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

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(-)
Show changes to t/t5520-pull.sh +1 −1
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
Ramkumar Ramachandra· Mar 22, 2013, 12:31 UTC · re: Ramkumar Ramachandra · lore

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

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(-)
Show changes to t/t5520-pull.sh +9 −13
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
Junio C Hamano· Mar 25, 2013, 15:59 UTC · re: Ramkumar Ramachandra · lore

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

Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 24 quoted lines
> 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.

Show 46 quoted lines
> @@ -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 &&
Ramkumar Ramachandra· Mar 28, 2013, 12:40 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to t/t5520-pull.sh +7 −11
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
Junio C Hamano· Mar 28, 2013, 14:50 UTC · re: Ramkumar Ramachandra · lore

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

Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 7 quoted lines
> 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?

Ramkumar Ramachandra· Mar 28, 2013, 14:57 UTC · re: Junio C Hamano · lore

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

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.
Junio C Hamano· Mar 28, 2013, 15:17 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> 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.

← back to recent threads