Re: [PATCH 08/16] t/t5516-fetch-push: use test_config()
Am 22.06.2013 00:32, schrieb Junio C Hamano: > Ramkumar Ramachandra 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 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()
Ramkumar Ramachandra 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 > --- > 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 2>stderr && > 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 2>stderr && > ! 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
[PATCH 08/16] t/t5516-fetch-push: use test_config()
Replace the 'git config' calls in tests with test_config for greater robustness. Signed-off-by: Ramkumar Ramachandra --- 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 2>stderr && 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 2>stderr && ! 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 2>stderr && ! grep "warning: updating the current branch" stderr @@ -918,7 +918,7 @@ test_expect_success 'push into aliased refs (consistent)' ' cd child1 && git branch foo && git symbolic-ref refs/heads/bar refs/heads/foo - git config receive.denyCurrentBranch false + test_config receive.denyCurrentBranch false ) && ( cd child2 && @@ -940,7 +940,7 @@ test_expect_success 'push into aliased refs (inconsistent)' ' cd child1 && git branch foo && git symbolic-ref refs/heads/bar refs/heads/foo -
