threads / patch / 35986

v2, 2 partst3200-branch: test setting branch as own upstream

Subject: [PATCH v2 1/2] t3200-branch: test setting branch as own upstream

## tl;dr

5 messages between Feb 28, 2014 and Mar 6, 2014. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Brian Gesiak· Feb 28, 2014, 06:41 UTC · lore

No test asserts that "git branch -u refs/heads/my-branch my-branch" emits a warning. Add a test that does so.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 t/t3200-branch.sh | 8 ++++++++
 1 file changed, 8 insertions(+)
Show changes to t/t3200-branch.sh +8 −0
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..6164126 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,6 +507,14 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
+	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
+	cat >expected <<EOF &&
+warning: Not setting branch my13 as its own upstream.
+EOF
+	test_i18ncmp expected actual
+'
+
 # Keep this test last, as it changes the current branch
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
-- 
1.8.3.4 (Apple Git-47)
Junio C Hamano· Mar 4, 2014, 23:56 UTC · re: Brian Gesiak · lore

Re: [PATCH v2 1/2] t3200-branch: test setting branch as own upstream

Brian Gesiak <modocache@gmail.com> writes:
Show 24 quoted lines
> No test asserts that "git branch -u refs/heads/my-branch my-branch"
> emits a warning. Add a test that does so.
>
> Signed-off-by: Brian Gesiak <modocache@gmail.com>
> ---
>  t/t3200-branch.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index fcdb867..6164126 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -507,6 +507,14 @@ EOF
>  	test_cmp expected actual
>  '
>  
> +test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
> +	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
> +	cat >expected <<EOF &&
> +warning: Not setting branch my13 as its own upstream.
> +EOF
> +	test_i18ncmp expected actual
> +'
> +

Checking the error message is fine, but we are also interested in seeing that we do not leave such a nonsense configuration, if not more. Shouldn't we check the resulting config as well here?

>  # Keep this test last, as it changes the current branch
>  cat >expect <<EOF
>  $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
Brian Gesiak· Mar 5, 2014, 07:31 UTC · re: Junio C Hamano · lore

[PATCH] t3200-branch: test setting branch as own upstream

No test asserts that "git branch -u refs/heads/my-branch my-branch" emits a warning. Add a test that does so.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 t/t3200-branch.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)
Show changes to t/t3200-branch.sh +10 −0
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..e6d4015 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,6 +507,16 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
+	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
+	cat >expected <<EOF &&
+warning: Not setting branch my13 as its own upstream.
+EOF
+	test_i18ncmp expected actual &&
+	test_must_fail git config branch.my13.remote &&
+	test_must_fail git config branch.my13.merge
+'
+
 # Keep this test last, as it changes the current branch
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
-- 
1.8.3.4 (Apple Git-47)
Jeff King· Mar 6, 2014, 21:00 UTC · re: Brian Gesiak · lore

Re: [PATCH] t3200-branch: test setting branch as own upstream

On Wed, Mar 05, 2014 at 04:31:55PM +0900, Brian Gesiak wrote:
> No test asserts that "git branch -u refs/heads/my-branch my-branch"
> emits a warning. Add a test that does so.
> 
> Signed-off-by: Brian Gesiak <modocache@gmail.com>

Thanks, this looks good. Two minor points that may or may not be worth addressing:

Show 5 quoted lines
> +test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
> +	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
> +	cat >expected <<EOF &&
> +warning: Not setting branch my13 as its own upstream.
> +EOF
If you spell the EOF marker as:
    cat >expect <<-\EOF
then:
  1. The shell does not interpolate the contents (it does not matter
     here, but it is a good habit to be in, so we typically do it unless
     there is a need to interpolate).
  2. Using <<- will strip leading tabs, so the content can be indented
     properly along with the rest of the test.
> +	test_i18ncmp expected actual &&
> +	test_must_fail git config branch.my13.remote &&
> +	test_must_fail git config branch.my13.merge
I think we could tighten these to:
  test_expect_code 1 git config branch.my13.remote

to eliminate a false-positive success on other config errors. It's highly improbable for it to ever matter, though (and it looks like we are not so careful in most other places that call "git config" looking for a missing entry, either).

-Peff
Junio C Hamano· Mar 6, 2014, 21:53 UTC · re: Jeff King · lore

Re: [PATCH] t3200-branch: test setting branch as own upstream

Jeff King <peff@peff.net> writes:
Show 41 quoted lines
> On Wed, Mar 05, 2014 at 04:31:55PM +0900, Brian Gesiak wrote:
>
>> No test asserts that "git branch -u refs/heads/my-branch my-branch"
>> emits a warning. Add a test that does so.
>> 
>> Signed-off-by: Brian Gesiak <modocache@gmail.com>
>
> Thanks, this looks good. Two minor points that may or may not be worth
> addressing:
>
>> +test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
>> +	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
>> +	cat >expected <<EOF &&
>> +warning: Not setting branch my13 as its own upstream.
>> +EOF
>
> If you spell the EOF marker as:
>
>     cat >expect <<-\EOF
>
> then:
>
>   1. The shell does not interpolate the contents (it does not matter
>      here, but it is a good habit to be in, so we typically do it unless
>      there is a need to interpolate).
>
>   2. Using <<- will strip leading tabs, so the content can be indented
>      properly along with the rest of the test.
>
>> +	test_i18ncmp expected actual &&
>> +	test_must_fail git config branch.my13.remote &&
>> +	test_must_fail git config branch.my13.merge
>
> I think we could tighten these to:
>
>   test_expect_code 1 git config branch.my13.remote
>
> to eliminate a false-positive success on other config errors. It's
> highly improbable for it to ever matter, though (and it looks like we
> are not so careful in most other places that call "git config" looking
> for a missing entry, either).
Sounds good.  Here is what I'll re-queue.
-- >8 --
From: Brian Gesiak <modocache@gmail.com>
Date: Wed, 5 Mar 2014 16:31:55 +0900
Subject: [PATCH] t3200-branch: test setting branch as own upstream

No test asserts that "git branch -u refs/heads/my-branch my-branch" avoids leaving nonsense configuration and emits a warning.

Add a test that does so.
Signed-off-by: Brian Gesiak <modocache@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3200-branch.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)
Show changes to t/t3200-branch.sh +10 −0
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..83037b1 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,6 +507,16 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success '--set-upstream-to notices an error to set branch as own upstream' '
+	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
+	cat >expected <<-\EOF &&
+	warning: Not setting branch my13 as its own upstream.
+	EOF
+	test_expect_code 1 git config branch.my13.remote &&
+	test_expect_code 1 git config branch.my13.merge &&
+	test_i18ncmp expected actual
+'
+
 # Keep this test last, as it changes the current branch
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
-- 
1.9.0-192-g8dd89d4

← back to recent threads