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

2014-03-06 Thread Jeff King
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 modoca...@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 2actual 
 + 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-03-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 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 modoca...@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 2actual 
 +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 modoca...@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 modoca...@gmail.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t3200-branch.sh | 10 ++
 1 file changed, 10 insertions(+)

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 2actual 
+   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 +
branch: Created from master
-- 
1.9.0-192-g8dd89d4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html