From 3c695523384855dffba49aec7f0a77e555ced822 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 18 Mar 2013 16:12:11 -0700 Subject: [PATCH 1/3] push test: use test_config when 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. This changes the meaning of some of the tests. For example, currently "push with insteadOf" passes even if the line setting "url.$TRASH.pushInsteadOf" is dropped because an url.$TRASH.insteadOf setting leaks in from a previous test. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index c31e5c1c52..5b89c111f1 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -203,7 +203,7 @@ test_expect_success 'push with wildcard' ' test_expect_success 'push with insteadOf' ' mk_empty && TRASH="$(pwd)/" && - git config "url.$TRASH.insteadOf" trash/ && + test_config "url.$TRASH.insteadOf" trash/ && git push trash/testrepo refs/heads/master:refs/remotes/origin/master && ( cd testrepo && @@ -217,7 +217,7 @@ test_expect_success 'push with insteadOf' ' test_expect_success 'push with pushInsteadOf' ' mk_empty && TRASH="$(pwd)/" && - git config "url.$TRASH.pushInsteadOf" trash/ && + test_config "url.$TRASH.pushInsteadOf" trash/ && git push trash/testrepo refs/heads/master:refs/remotes/origin/master && ( cd testrepo && @@ -231,9 +231,9 @@ test_expect_success 'push with pushInsteadOf' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty && TRASH="$(pwd)/" && - git config "url.trash2/.pushInsteadOf" trash/ && - git config remote.r.url trash/wrong && - git config remote.r.pushurl "$TRASH/testrepo" && + test_config "url.trash2/.pushInsteadOf" trash/ && + test_config remote.r.url trash/wrong && + test_config remote.r.pushurl "$TRASH/testrepo" && git push r refs/heads/master:refs/remotes/origin/master && ( cd testrepo && @@ -489,31 +489,24 @@ test_expect_success 'push with config remote.*.push = HEAD' ' git checkout local && git reset --hard $the_first_commit ) && - git config remote.there.url testrepo && - git config remote.there.push HEAD && - git config branch.master.remote there && + test_config remote.there.url testrepo && + test_config remote.there.push HEAD && + test_config branch.master.remote there && git push && check_push_result $the_commit heads/master && check_push_result $the_first_commit heads/local ' -# clean up the cruft left with the previous one -git config --remove-section remote.there -git config --remove-section branch.master - test_expect_success 'push with config remote.*.pushurl' ' mk_test heads/master && git checkout master && - git config remote.there.url test2repo && - git config remote.there.pushurl testrepo && + test_config remote.there.url test2repo && + test_config remote.there.pushurl testrepo && git push there && check_push_result $the_commit heads/master ' -# clean up the cruft left with the previous one -git config --remove-section remote.there - test_expect_success 'push with dry-run' ' mk_test heads/master && From 848575d833ca0d9d47db372348576f3b98bd19b7 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 18 Mar 2013 16:13:41 -0700 Subject: [PATCH 2/3] push test: simplify check of push result This test checks each ref with code like the following: r=$(git show-ref -s --verify refs/$ref) && test "z$r" = "z$the_first_commit" Afterward it counts refs: test 1 = $(git for-each-ref refs/remotes/origin | wc -l) Simpler to test the number and values of relevant refs in for-each-ref output at the same time using test_cmp. This makes the test more readable and provides more helpful "./t5516-push-push.sh -v" output when the test fails. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 114 +++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 63 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 5b89c111f1..2f1255d467 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -30,11 +30,10 @@ mk_test () { cd testrepo && for ref in "$@" do - r=$(git show-ref -s --verify refs/$ref) && - test "z$r" = "z$the_first_commit" || { - echo "Oops, refs/$ref is wrong" - exit 1 - } + echo "$the_first_commit" >expect && + git show-ref -s --verify refs/$ref >actual && + test_cmp expect actual || + exit done && git fsck --full ) @@ -82,15 +81,13 @@ mk_child() { check_push_result () { ( cd testrepo && - it="$1" && - shift + echo "$1" >expect && + shift && for ref in "$@" do - r=$(git show-ref -s --verify refs/$ref) && - test "z$r" = "z$it" || { - echo "Oops, refs/$ref is wrong" - exit 1 - } + git show-ref -s --verify refs/$ref >actual && + test_cmp expect actual || + exit done && git fsck --full ) @@ -118,10 +115,9 @@ test_expect_success 'fetch without wildcard' ' cd testrepo && git fetch .. refs/heads/master:refs/remotes/origin/master && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -133,10 +129,9 @@ test_expect_success 'fetch with wildcard' ' git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" && git fetch up && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -150,10 +145,9 @@ test_expect_success 'fetch with insteadOf' ' git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" && git fetch up && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -167,10 +161,9 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' ' git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" && git fetch up && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -180,10 +173,9 @@ test_expect_success 'push without wildcard' ' git push testrepo refs/heads/master:refs/remotes/origin/master && ( cd testrepo && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -193,10 +185,9 @@ test_expect_success 'push with wildcard' ' git push testrepo "refs/heads/*:refs/remotes/origin/*" && ( cd testrepo && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -207,10 +198,9 @@ test_expect_success 'push with insteadOf' ' git push trash/testrepo refs/heads/master:refs/remotes/origin/master && ( cd testrepo && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -221,10 +211,9 @@ test_expect_success 'push with pushInsteadOf' ' git push trash/testrepo refs/heads/master:refs/remotes/origin/master && ( cd testrepo && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -237,10 +226,9 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf git push r refs/heads/master:refs/remotes/origin/master && ( cd testrepo && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) ' @@ -827,9 +815,9 @@ test_expect_success 'fetch with branches' ' ( cd testrepo && git fetch branch1 && - r=$(git show-ref -s --verify refs/heads/branch1) && - test "z$r" = "z$the_commit" && - test 1 = $(git for-each-ref refs/heads | wc -l) + echo "$the_commit commit refs/heads/branch1" >expect && + git for-each-ref refs/heads >actual && + test_cmp expect actual ) && git checkout master ' @@ -840,9 +828,9 @@ test_expect_success 'fetch with branches containing #' ' ( cd testrepo && git fetch branch2 && - r=$(git show-ref -s --verify refs/heads/branch2) && - test "z$r" = "z$the_first_commit" && - test 1 = $(git for-each-ref refs/heads | wc -l) + echo "$the_first_commit commit refs/heads/branch2" >expect && + git for-each-ref refs/heads >actual && + test_cmp expect actual ) && git checkout master ' @@ -854,9 +842,9 @@ test_expect_success 'push with branches' ' git push branch1 && ( cd testrepo && - r=$(git show-ref -s --verify refs/heads/master) && - test "z$r" = "z$the_first_commit" && - test 1 = $(git for-each-ref refs/heads | wc -l) + echo "$the_first_commit commit refs/heads/master" >expect && + git for-each-ref refs/heads >actual && + test_cmp expect actual ) ' @@ -866,9 +854,9 @@ test_expect_success 'push with branches containing #' ' git push branch2 && ( cd testrepo && - r=$(git show-ref -s --verify refs/heads/branch3) && - test "z$r" = "z$the_first_commit" && - test 1 = $(git for-each-ref refs/heads | wc -l) + echo "$the_first_commit commit refs/heads/branch3" >expect && + git for-each-ref refs/heads >actual && + test_cmp expect actual ) && git checkout master ' @@ -951,9 +939,9 @@ test_expect_success 'push --porcelain' ' git push >.git/bar --porcelain testrepo refs/heads/master:refs/remotes/origin/master && ( cd testrepo && - r=$(git show-ref -s --verify refs/remotes/origin/master) && - test "z$r" = "z$the_commit" && - test 1 = $(git for-each-ref refs/remotes/origin | wc -l) + echo "$the_commit commit refs/remotes/origin/master" >expect && + git for-each-ref refs/remotes/origin >actual && + test_cmp expect actual ) && test_cmp .git/foo .git/bar ' From 5bd81c73156a623edc9dff42c26469732f49aa89 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 18 Mar 2013 16:14:26 -0700 Subject: [PATCH 3/3] push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi' When it is unclear which command from a test has failed, usual practice these days is to debug by running the test again with "sh -x" instead of relying on debugging 'echo' statements. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 2f1255d467..0695d570f2 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -22,10 +22,8 @@ mk_test () { ( for ref in "$@" do - git push testrepo $the_first_commit:refs/$ref || { - echo "Oops, push refs/$ref failure" - exit 1 - } + git push testrepo $the_first_commit:refs/$ref || + exit done && cd testrepo && for ref in "$@" @@ -328,13 +326,8 @@ test_expect_success 'push with weak ambiguity (2)' ' test_expect_success 'push with ambiguity' ' mk_test heads/frotz tags/frotz && - if git push testrepo master:frotz - then - echo "Oops, should have failed" - false - else - check_push_result $the_first_commit heads/frotz tags/frotz - fi + test_must_fail git push testrepo master:frotz && + check_push_result $the_first_commit heads/frotz tags/frotz '