Re: [PATCH 08/16] t/t5516-fetch-push: use test_config()

[Ramkumar Ramachandra]

Junio C Hamano wrote:
 That may be a good thing in principle, but I _think_
 [...]
 sequence is used so that we do not even have to worry about what
 leftover configuration values are in the testrepo/.git/config; so
 does it really matter?

Yeah, you're right.  Dropped.
--
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 08/16] t/t5516-fetch-push: use test_config()

[Johannes Sixt]

Am 22.06.2013 00:32, schrieb Junio C Hamano:
 Ramkumar Ramachandra artag...@gmail.com writes:
 
 Replace the 'git config' calls in tests with test_config for greater
 robustness.
 
 That may be a good thing in principle, but I _think_
 
   mk_empty testrepo 
 (
   cd testrepo 
 do whatever to its config 
 run test
   )
 
 sequence is used so that we do not even have to worry about what
 leftover configuration values are in the testrepo/.git/config; so
 does it really matter?
 
 If this conversion had something more than s/git config/test_config/
 replacement, that would indicate that you uncovered a bug in the
 existing test and found a good fix, but that does not seem to be the
 case for this particular patch.

And just let me add that the added benefit of test_config to remove the
configuration change after the test case finished would not work because
the test_when_finished registration that happens behind the scenes would
be forgotten when the sub-shell exits.

 @@ -142,8 +142,8 @@ test_expect_success 'fetch with wildcard' '
  mk_empty testrepo 
  (
  cd testrepo 
 -git config remote.up.url .. 
 -git config remote.up.fetch refs/heads/*:refs/remotes/origin/* 
 
 +test_config remote.up.url .. 
 +test_config remote.up.fetch 
 refs/heads/*:refs/remotes/origin/* 
  git fetch up 
...

-- Hannes

--
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 08/16] t/t5516-fetch-push: use test_config()

[Junio C Hamano]

Ramkumar Ramachandra artag...@gmail.com writes:

 Replace the 'git config' calls in tests with test_config for greater
 robustness.

That may be a good thing in principle, but I _think_

mk_empty testrepo 
(
cd testrepo 
do whatever to its config 
run test
)

sequence is used so that we do not even have to worry about what
leftover configuration values are in the testrepo/.git/config; so
does it really matter?

If this conversion had something more than s/git config/test_config/
replacement, that would indicate that you uncovered a bug in the
existing test and found a good fix, but that does not seem to be the
case for this particular patch.


 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  t/t5516-fetch-push.sh | 46 +++---
  1 file changed, 23 insertions(+), 23 deletions(-)

 diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
 index 6e9fa84..afb25c4 100755
 --- a/t/t5516-fetch-push.sh
 +++ b/t/t5516-fetch-push.sh
 @@ -142,8 +142,8 @@ test_expect_success 'fetch with wildcard' '
   mk_empty testrepo 
   (
   cd testrepo 
 - git config remote.up.url .. 
 - git config remote.up.fetch refs/heads/*:refs/remotes/origin/* 
 
 + test_config remote.up.url .. 
 + test_config remote.up.fetch 
 refs/heads/*:refs/remotes/origin/* 
   git fetch up 
  
   echo $the_commit commitrefs/remotes/origin/master 
 expect 
 @@ -157,9 +157,9 @@ test_expect_success 'fetch with insteadOf' '
   (
   TRASH=$(pwd)/ 
   cd testrepo 
 - git config url.$TRASH.insteadOf trash/ 
 - git config remote.up.url trash/. 
 - git config remote.up.fetch refs/heads/*:refs/remotes/origin/* 
 
 + test_config url.$TRASH.insteadOf trash/ 
 + test_config remote.up.url trash/. 
 + test_config remote.up.fetch 
 refs/heads/*:refs/remotes/origin/* 
   git fetch up 
  
   echo $the_commit commitrefs/remotes/origin/master 
 expect 
 @@ -173,9 +173,9 @@ test_expect_success 'fetch with pushInsteadOf (should not 
 rewrite)' '
   (
   TRASH=$(pwd)/ 
   cd testrepo 
 - git config url.trash/.pushInsteadOf $TRASH 
 - git config remote.up.url $TRASH. 
 - git config remote.up.fetch refs/heads/*:refs/remotes/origin/* 
 
 + test_config url.trash/.pushInsteadOf $TRASH 
 + test_config remote.up.url $TRASH. 
 + test_config remote.up.fetch 
 refs/heads/*:refs/remotes/origin/* 
   git fetch up 
  
   echo $the_commit commitrefs/remotes/origin/master 
 expect 
 @@ -780,7 +780,7 @@ test_expect_success 'mixed ref updates, deletes, invalid 
 deletes trigger hooks w
  
  test_expect_success 'allow deleting a ref using --delete' '
   mk_test testrepo heads/master 
 - (cd testrepo  git config receive.denyDeleteCurrent warn) 
 + (cd testrepo  test_config receive.denyDeleteCurrent warn) 
   git push testrepo --delete master 
   (cd testrepo  test_must_fail git rev-parse --verify refs/heads/master)
  '
 @@ -809,7 +809,7 @@ test_expect_success 'warn on push to HEAD of non-bare 
 repository' '
   (
   cd testrepo 
   git checkout master 
 - git config receive.denyCurrentBranch warn
 + test_config receive.denyCurrentBranch warn
   ) 
   git push testrepo master 2stderr 
   grep warning: updating the current branch stderr
 @@ -820,7 +820,7 @@ test_expect_success 'deny push to HEAD of non-bare 
 repository' '
   (
   cd testrepo 
   git checkout master 
 - git config receive.denyCurrentBranch true
 + test_config receive.denyCurrentBranch true
   ) 
   test_must_fail git push testrepo master
  '
 @@ -830,8 +830,8 @@ test_expect_success 'allow push to HEAD of bare 
 repository (bare)' '
   (
   cd testrepo 
   git checkout master 
 - git config receive.denyCurrentBranch true 
 - git config core.bare true
 + test_config receive.denyCurrentBranch true 
 + test_config core.bare true
   ) 
   git push testrepo master 2stderr 
   ! grep warning: updating the current branch stderr
 @@ -842,7 +842,7 @@ test_expect_success 'allow push to HEAD of non-bare 
 repository (config)' '
   (
   cd testrepo 
   git checkout master 
 - git config receive.denyCurrentBranch false
 + test_config receive.denyCurrentBranch false
   ) 
   git push testrepo master 2stderr 
   ! grep warning: updating the current branch stderr
 @@ -918,7 +918,7 @@ test_expect_success 'push into aliased refs (consistent)' 
 '
   cd child1