{"thread":{"id":"39223","subject":"[PATCH 1/7] t5520: test pulling multiple branches into an empty repository","startedAt":"2015-05-02T15:37:08Z","lastAt":"2015-05-06T06:30:44Z","messageCount":21,"participants":["Paul Tan","Eric Sunshine","Matthieu Moy","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"260429","messageId":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":null,"subject":"[PATCH 0/7] Improve git-pull test coverage","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:08Z","receivedAt":"2015-05-02T15:37:08Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"This patch series improves test coverage of git-pull.sh from 50.9%[1] to\n73.4%[2]. Take the test coverage reports with a grain of salt though, as there\nare known deficiencies[3].\n\nThis is part of my GSoC project to rewrite git-pull into a builtin. Improving\ntest coverage helps to prevent regressions that could occur due to the rewrite.\n\n[1] http://pyokagan.github.io/git/20150430132408-a75942b//kcov-merged/git-pull.360f32c2.html\n[2] http://pyokagan.github.io/git/20150502145725-c94dfc2//kcov-merged/git-pull.360f32c2.html\n[3] https://github.com/pyokagan/git/commit/a7d6ab4677b97afd789b0ce860280c80f70f6e32\n\nPaul Tan (7):\n  t5520: test pulling multiple branches into an empty repository\n  t5520: implement tests for no merge candidates cases\n  t5520: test for failure if index has unresolved entries\n  t5520: test work tree fast-forward when fetch updates head\n  t5520: test --rebase with multiple branches\n  t5520: test --rebase failure on unborn branch with index\n  t5521: test --dry-run does not make any changes\n\n t/t5520-pull.sh         | 119 ++++++++++++++++++++++++++++++++++++++++++++++++\n t/t5521-pull-options.sh |  13 ++++++\n 2 files changed, 132 insertions(+)\n\n-- \n2.1.4\n"},{"id":"260428","messageId":"1430581035-29464-2-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 1/7] t5520: test pulling multiple branches into an empty repository","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:09Z","receivedAt":"2015-05-02T15:37:09Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit d09e79cb (\"allow pulling into an empty repository\") introduced\nthe ability to pull into an empty head. As pulling in multiple branches\ndoes not make sense in this context, git-pull explicitly fails when\nthere is an empty head and multiple branches are specified.\n\nAdd a test to ensure that this safeguard works.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/t5520-pull.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 227d293..01ae1bf 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -86,6 +86,18 @@ test_expect_success 'pulling into void does not remove new staged files' '\n \t)\n '\n \n+test_expect_success 'refuse to pull multiple branches into void' '\n+\tgit branch test master &&\n+\ttest_when_finished \"git branch -D test\" &&\n+\tgit init cloned-multiple-branches &&\n+\ttest_when_finished \"rm -rf cloned-multiple-branches\" &&\n+\t(\n+\t\tcd cloned-multiple-branches &&\n+\t\ttest_must_fail git pull .. master test 2>out &&\n+\t\ttest_i18ngrep \"Cannot merge multiple branches into empty head\" out\n+\t)\n+'\n+\n test_expect_success 'test . as a remote' '\n \n \tgit branch copy master &&\n-- \n2.1.4\n"},{"id":"260430","messageId":"1430581035-29464-3-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 2/7] t5520: implement tests for no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:10Z","receivedAt":"2015-05-02T15:37:10Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit a8c9bef4 fully established the current advices given by git-pull\nfor the different cases where git-fetch will not have anything marked\nfor merge:\n\n1. We're not on a branch, so there is no branch\n   configuration.\n\n2. We're on a branch, but there is no configuration for\n   this branch.\n\n3. We fetched from the configured remote, but the\n   configured branch to merge didn't get fetched (either\n   it doesn't exist, or wasn't part of the fetch refspec).\n\n4. We fetched from the non-default remote, but didn't\n   specify a branch to merge. We can't use the configured\n   one because it applies to the default remote.\n\n5. We fetched from a specified remote, and a refspec was\n   given, but it ended up not fetching anything.\n\nImplement tests for the above 5 cases to ensure that the correct code\npaths are triggered for each of these cases.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nI'm having trouble hitting the 1st case without resorting to the hack below. A\ndetached HEAD will always have no remote configured, and the code flow would\nmake it such that case (4) is hit in the detached HEAD case instead of case\n(1).\n\n t/t5520-pull.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 52 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 01ae1bf..635ec02 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -122,6 +122,58 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \ttest `cat file` = modified\n '\n \n+test_expect_success 'fail if not on a branch' '\n+\tcp .git/config .git/config.bak &&\n+\ttest_when_finished \"cp .git/config.bak .git/config\" &&\n+\tgit remote add test_remote . &&\n+\tgit checkout HEAD^{} &&\n+\ttest_when_finished \"git checkout -f copy\" &&\n+\tcat >>.git/config <<-\\EOF &&\n+\t[branch \"\"]\n+\tremote = test_remote\n+\tEOF\n+\ttest_must_fail git pull test_remote 2>out &&\n+\ttest_i18ngrep \"You are not currently on a branch\" out\n+'\n+\n+test_expect_success 'fail if no configuration for current branch' '\n+\tgit remote add test_remote . &&\n+\ttest_when_finished \"git remote remove test_remote\" &&\n+\tgit checkout -b test copy &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n+\ttest_config branch.test.remote test_remote &&\n+\ttest_must_fail git pull 2>out &&\n+\ttest_i18ngrep \"There is no tracking information\" out\n+'\n+\n+test_expect_success 'fail if upstream branch does not exist' \"\n+\tgit checkout -b test copy &&\n+\ttest_when_finished 'git checkout -f copy && git branch -D test' &&\n+\ttest_config branch.test.remote . &&\n+\ttest_config branch.test.merge refs/heads/nonexisting &&\n+\ttest_must_fail git pull 2>out &&\n+\ttest_i18ngrep \\\"Your configuration specifies to merge with the ref 'nonexisting'\\\" out\n+\"\n+\n+test_expect_success 'fail if no branches specified with non-default remote' '\n+\tgit clone --bare . test_repo &&\n+\ttest_when_finished \"rm -rf test_repo\" &&\n+\tgit remote add test_remote test_repo &&\n+\ttest_when_finished \"git remote remove test_remote\" &&\n+\tgit checkout -b test master &&\n+\ttest_when_finished \"git checkout -f master && git branch -D test\" &&\n+\ttest_config branch.test.remote . &&\n+\ttest_must_fail git pull test_remote 2>out &&\n+\ttest_i18ngrep \"you must specify a branch on the command line\" out\n+'\n+\n+test_expect_success 'fail if wildcard spec does not match any refs' \"\n+\tgit checkout -b test copy &&\n+\ttest_when_finished 'git checkout -f copy && git branch -D test' &&\n+\ttest_must_fail git pull . 'refs/nonexisting1/*:refs/nonexisting2/*' 2>out &&\n+\ttest_i18ngrep 'There are no candidates for merging' out\n+\"\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"260433","messageId":"1430581035-29464-4-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 3/7] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:11Z","receivedAt":"2015-05-02T15:37:11Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit d38a30df (Be more user-friendly when refusing to do something\nbecause of conflict) introduced code paths to git-pull which will error\nout with user-friendly advices if the user is in the middle of a merge\nor has unmerged files.\n\nImplement tests to ensure that git-pull will not run, and will print\nthese advices, if the user is in the middle of a merge or has unmerged\nfiles in the index.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/t5520-pull.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 635ec02..9f57e0d 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -174,6 +174,21 @@ test_expect_success 'fail if wildcard spec does not match any refs' \"\n \ttest_i18ngrep 'There are no candidates for merging' out\n \"\n \n+test_expect_success 'fail if the index has unresolved entries' '\n+\tgit checkout -b third master^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\techo file >expected &&\n+\ttest_cmp expected file &&\n+\techo modified2 >file &&\n+\tgit commit -a -m modified2 &&\n+\ttest_must_fail git pull . second &&\n+\ttest_must_fail git pull . second 2>out &&\n+\ttest_i18ngrep \"Pull is not possible because you have unmerged files\" out &&\n+\tgit add file &&\n+\ttest_must_fail git pull . second 2>out &&\n+\ttest_i18ngrep \"You have not concluded your merge\" out\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"260432","messageId":"1430581035-29464-5-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 4/7] t5520: test work tree fast-forward when fetch updates head","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:12Z","receivedAt":"2015-05-02T15:37:12Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since commit b10ac50f (Fix pulling into the same branch), git-pull,\nupon detecting that git-fetch updated the current head, will\nfast-forward the working tree to the updated head commit.\n\nImplement tests to ensure that the fast-forward occurs in such a case,\nas well as to ensure that the user-friendly advice is printed upon\nfailure.\n---\n t/t5520-pull.sh | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 9f57e0d..25d519d 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -189,6 +189,27 @@ test_expect_success 'fail if the index has unresolved entries' '\n \ttest_i18ngrep \"You have not concluded your merge\" out\n '\n \n+test_expect_success 'fast-forwards working tree if branch head is updated' '\n+\tgit checkout -b third master^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\techo file >expected &&\n+\ttest_cmp expected file &&\n+\tgit pull . second:third 2>out &&\n+\ttest_i18ngrep \"fetch updated the current branch head\" out &&\n+\techo modified >expected &&\n+\ttest_cmp expected file\n+'\n+\n+test_expect_success 'fast-forward fails with conflicting work tree' '\n+\tgit checkout -b third master^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\techo file >expected &&\n+\ttest_cmp expected file &&\n+\techo conflict >file &&\n+\ttest_must_fail git pull . second:third 2>out &&\n+\ttest_i18ngrep \"Cannot fast-forward your working tree\" out\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"260431","messageId":"1430581035-29464-6-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 5/7] t5520: test --rebase with multiple branches","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:13Z","receivedAt":"2015-05-02T15:37:13Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since rebasing on top of multiple upstream branches does not make sense,\nsince commit 51b2ead0 (\"disallow providing multiple upstream branches\nto rebase, pull --rebase\"), git-pull explicitly disallowed specifying\nmultiple branches in the rebase case.\n\nImplement tests to ensure that git-pull fails and prints out the\nuser-friendly error message in such a case.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/t5520-pull.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 25d519d..17c63ff 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -223,6 +223,14 @@ test_expect_success '--rebase' '\n \ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n \ttest new = $(git show HEAD:file2)\n '\n+\n+test_expect_success '--rebase fails with multiple branches' '\n+\tgit reset --hard before-rebase &&\n+\ttest_must_fail git pull --rebase . copy master 2>out &&\n+\ttest_when_finished \"rm -f out\" &&\n+\ttest_i18ngrep \"Cannot rebase onto multiple branches\" out\n+'\n+\n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config pull.rebase true &&\n-- \n2.1.4\n"},{"id":"260434","messageId":"1430581035-29464-7-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 6/7] t5520: test --rebase failure on unborn branch with index","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:14Z","receivedAt":"2015-05-02T15:37:14Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit 19a7fcbf (allow pull --rebase on branch yet to be born) special\ncases git-pull on an unborn branch in a different code path such that\ngit-pull --rebase is still valid even though there is no HEAD yet.\n\nThis code path still ensures that there is no index in order not to lose\nany staged changes. Implement a test to ensure that this check is\ntriggered.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nI wonder if the .git/index check is still required, as git-read-tree -m\nwill take into account entries in the index as well.\n\n t/t5520-pull.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 17c63ff..34f3780 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -417,6 +417,17 @@ test_expect_success 'pull --rebase works on branch yet to be born' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n+\tgit init empty_repo2 &&\n+\ttest_when_finished \"rm -rf empty_repo2\" &&\n+\t(\n+\t\tcd empty_repo2 &&\n+\t\techo staged-file >staged-file &&\n+\t\tgit add staged-file &&\n+\t\ttest_must_fail git pull --rebase .. master\n+\t)\n+'\n+\n test_expect_success 'setup for detecting upstreamed changes' '\n \tmkdir src &&\n \t(cd src &&\n-- \n2.1.4\n"},{"id":"260435","messageId":"1430581035-29464-8-git-send-email-pyokagan@gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 7/7] t5521: test --dry-run does not make any changes","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-02T15:37:15Z","receivedAt":"2015-05-02T15:37:15Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Test that when --dry-run is provided to git-pull, it does not make any\nchanges, namely:\n\n* --dry-run gets passed to git-fetch, so no FETCH_HEAD will be created\n  and no refs will be fetched.\n\n* The index and work tree will not be modified.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/t5521-pull-options.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 453aba5..7c4d624 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -117,4 +117,17 @@ test_expect_success 'git pull --all' '\n \t)\n '\n \n+test_expect_success 'git pull --dry-run' '\n+\tgit init clonedry &&\n+\ttest_when_finished \"rm -rf clonedry\" &&\n+\t(\n+\t\tcd clonedry &&\n+\t\tgit pull --dry-run \"../parent\" &&\n+\t\ttest_path_is_missing .git/FETCH_HEAD &&\n+\t\ttest_path_is_missing .git/refs/heads/master &&\n+\t\ttest_path_is_missing .git/index &&\n+\t\ttest_path_is_missing \"file\"\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"260453","messageId":"CAPig+cTBm9iN8bFEZx3rHzYhwapSk4wYcAQXiqtu_BzxNd9VSA@mail.gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 4/7] t5520: test work tree fast-forward when fetch updates head","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-03T02:42:52Z","receivedAt":"2015-05-03T02:42:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, May 2, 2015 at 11:37 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> Since commit b10ac50f (Fix pulling into the same branch), git-pull,\n> upon detecting that git-fetch updated the current head, will\n> fast-forward the working tree to the updated head commit.\n>\n> Implement tests to ensure that the fast-forward occurs in such a case,\n> as well as to ensure that the user-friendly advice is printed upon\n> failure.\n\nMissing sign-off.\n\n> ---\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 9f57e0d..25d519d 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -189,6 +189,27 @@ test_expect_success 'fail if the index has unresolved entries' '\n>         test_i18ngrep \"You have not concluded your merge\" out\n>  '\n>\n> +test_expect_success 'fast-forwards working tree if branch head is updated' '\n> +       git checkout -b third master^ &&\n> +       test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> +       echo file >expected &&\n> +       test_cmp expected file &&\n> +       git pull . second:third 2>out &&\n> +       test_i18ngrep \"fetch updated the current branch head\" out &&\n> +       echo modified >expected &&\n> +       test_cmp expected file\n> +'\n> +\n> +test_expect_success 'fast-forward fails with conflicting work tree' '\n> +       git checkout -b third master^ &&\n> +       test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> +       echo file >expected &&\n> +       test_cmp expected file &&\n> +       echo conflict >file &&\n> +       test_must_fail git pull . second:third 2>out &&\n> +       test_i18ngrep \"Cannot fast-forward your working tree\" out\n> +'\n> +\n>  test_expect_success '--rebase' '\n>         git branch to-rebase &&\n>         echo modified again > file &&\n> --\n> 2.1.4\n"},{"id":"260452","messageId":"CACRoPnQx9g-VOP88=H19S5TqF6EkKcCFAQfTJ_6vvk2Qwsn5nA@mail.gmail.com","threadId":"39223","inReplyTo":"CAPig+cTBm9iN8bFEZx3rHzYhwapSk4wYcAQXiqtu_BzxNd9VSA@mail.gmail.com","subject":"Re: [PATCH 4/7] t5520: test work tree fast-forward when fetch updates head","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-03T02:47:53Z","receivedAt":"2015-05-03T02:47:53Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Sun, May 3, 2015 at 10:42 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> Missing sign-off.\n\nRight, whoops.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n"},{"id":"260508","messageId":"vpqvbg8u5sq.fsf@anie.imag.fr","threadId":"39223","inReplyTo":"1430581035-29464-3-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 2/7] t5520: implement tests for no merge candidates cases","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-04T08:04:53Z","receivedAt":"2015-05-04T08:04:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Thanks for the patch. On overall, it looks good to me. Some minor\ncomments below.\n\nPaul Tan <pyokagan@gmail.com> writes:\n\n> Commit a8c9bef4 fully established the current advices given by git-pull\n> for the different cases where git-fetch will not have anything marked\n> for merge:\n>\n> 1. We're not on a branch, so there is no branch\n>    configuration.\n\nNit: you seem to be wrapping your lines with a rather short length. I\nwould prefer reading this as a single line:\n\n1. We're not on a branch, so there is no branch configuration.\n\n(62 columns)\n\n> ---\n>\n> I'm having trouble hitting the 1st case without resorting to the hack below. A\n> detached HEAD will always have no remote configured, and the code flow would\n> make it such that case (4) is hit in the detached HEAD case instead of case\n> (1).\n\nThis should appear in comments in the test 'fail if not on a branch'.\nPeople reading your [branch \"\"] in the future won't look for\nbelow-triple-dash comments in the mailing-list archives ...\n\nAnd actually, it would be more user-friendly to trigger this error\nmessage in the normal senario, i.e. check for 1. before 4. in the code.\nThis was most likely the intension of the programmer who wrote this\nerror message. You may want to fix this now, or add a\ntest_expect_failure which will become a test_expect_success when you\nreplace git-pull.sh with builtin/pull.c.\n\n>  t/t5520-pull.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 52 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 01ae1bf..635ec02 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -122,6 +122,58 @@ test_expect_success 'the default remote . should not break explicit pull' '\n>  \ttest `cat file` = modified\n>  '\n>  \n> +test_expect_success 'fail if not on a branch' '\n> +\tcp .git/config .git/config.bak &&\n> +\ttest_when_finished \"cp .git/config.bak .git/config\" &&\n> +\tgit remote add test_remote . &&\n> +\tgit checkout HEAD^{} &&\n> +\ttest_when_finished \"git checkout -f copy\" &&\n> +\tcat >>.git/config <<-\\EOF &&\n> +\t[branch \"\"]\n> +\tremote = test_remote\n> +\tEOF\n> +\ttest_must_fail git pull test_remote 2>out &&\n> +\ttest_i18ngrep \"You are not currently on a branch\" out\n\nIt may make sense to grep only a subset of the string, to be less\nsensitive to rewords of error message. For example, just:\n\ntest_i18ngrep \"not currently on a branch\"\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"260509","messageId":"vpqmw1ku5l3.fsf@anie.imag.fr","threadId":"39223","inReplyTo":"1430581035-29464-4-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 3/7] t5520: test for failure if index has unresolved entries","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-04T08:09:28Z","receivedAt":"2015-05-04T08:09:28Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Commit d38a30df (Be more user-friendly when refusing to do something\n> because of conflict) introduced code paths to git-pull which will error\n> out with user-friendly advices if the user is in the middle of a merge\n> or has unmerged files.\n\nWhen referencing a commit like this, you may want to add the author of\nthe commit in Cc: to make sure he reads. I did read, so it's OK. And\nsorry for not testing my code enough ;-).\n\nThe patch looks good to me.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"260510","messageId":"vpqoam0sqp5.fsf@anie.imag.fr","threadId":"39223","inReplyTo":"1430581035-29464-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 0/7] Improve git-pull test coverage","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-04T08:16:22Z","receivedAt":"2015-05-04T08:16:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Paul Tan (7):\n>   t5520: test pulling multiple branches into an empty repository\n>   t5520: implement tests for no merge candidates cases\n>   t5520: test for failure if index has unresolved entries\n>   t5520: test work tree fast-forward when fetch updates head\n>   t5520: test --rebase with multiple branches\n>   t5520: test --rebase failure on unborn branch with index\n>   t5521: test --dry-run does not make any changes\n\nI did a semi-thourough review of the whole series, it looks good to me.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"260529","messageId":"CAGZ79kbSMbhW2FLK05CaQjJjSTqEurB8iyeEBgyA1wbWJ4g2vw@mail.gmail.com","threadId":"39223","inReplyTo":"1430581035-29464-6-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 5/7] t5520: test --rebase with multiple branches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-04T17:09:37Z","receivedAt":"2015-05-04T17:09:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, May 2, 2015 at 8:37 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> Since rebasing on top of multiple upstream branches does not make sense,\n> since commit 51b2ead0 (\"disallow providing multiple upstream branches\n> to rebase, pull --rebase\"), git-pull explicitly disallowed specifying\n> multiple branches in the rebase case.\n>\n> Implement tests to ensure that git-pull fails and prints out the\n> user-friendly error message in such a case.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>  t/t5520-pull.sh | 8 ++++++++\n>  1 file changed, 8 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 25d519d..17c63ff 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -223,6 +223,14 @@ test_expect_success '--rebase' '\n>         test $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n>         test new = $(git show HEAD:file2)\n>  '\n> +\n> +test_expect_success '--rebase fails with multiple branches' '\n> +       git reset --hard before-rebase &&\n> +       test_must_fail git pull --rebase . copy master 2>out &&\n> +       test_when_finished \"rm -f out\" &&\n\nI think it would make sense to switch the previous 2 lines, because the\ntest_when_finished should also be run if the test actually fails.\nThe actual tested part may segfault, which would interrupt the && chain\nfrom reaching the test_when_finished line. And here, the line to test\nis meant to be the \"git pull --rebase\" line, so we'd assume it may be\nstopping there due to failures in the tested program.\n\nIf you grep through the code base, you'll find lots of test_when_finished\ncommands as the very first command (or the earliest spot where it makes\nsense).\n\n> +       test_i18ngrep \"Cannot rebase onto multiple branches\" out\n> +'\n> +\n>  test_expect_success 'pull.rebase' '\n>         git reset --hard before-rebase &&\n>         test_config pull.rebase true &&\n> --\n> 2.1.4\n>\n"},{"id":"260531","messageId":"xmqqlhh4tfd0.fsf@gitster.dls.corp.google.com","threadId":"39223","inReplyTo":"vpqoam0sqp5.fsf@anie.imag.fr","subject":"Re: [PATCH 0/7] Improve git-pull test coverage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-04T17:35:55Z","receivedAt":"2015-05-04T17:35:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> Paul Tan (7):\n>>   t5520: test pulling multiple branches into an empty repository\n>>   t5520: implement tests for no merge candidates cases\n>>   t5520: test for failure if index has unresolved entries\n>>   t5520: test work tree fast-forward when fetch updates head\n>>   t5520: test --rebase with multiple branches\n>>   t5520: test --rebase failure on unborn branch with index\n>>   t5521: test --dry-run does not make any changes\n>\n> I did a semi-thourough review of the whole series, it looks good to me.\n\nThanks. I did the same, and it looked OK.\n\nOne test was duplicated with jc/merge series and caused a textual\nconflict, but that was nothing major.\n\nI didn't like the grepping of error messages alone as a way to\ndiagnose the failure, though.  When we expect the pull to fail\nwithout touching anything in the working tree, I'd prefer to see\nthat tested explicitly (e.g. \"if pull mistakenly tried to go ahead\nand touch this file that would be involved in the merge, its\ncontents would change---let's make sure that does not happen by\nmaking sure the contents before and after are the same\" kind of\nthing).\n"},{"id":"260540","messageId":"xmqqmw1krvqu.fsf@gitster.dls.corp.google.com","threadId":"39223","inReplyTo":"CAGZ79kbSMbhW2FLK05CaQjJjSTqEurB8iyeEBgyA1wbWJ4g2vw@mail.gmail.com","subject":"Re: [PATCH 5/7] t5520: test --rebase with multiple branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-04T19:24:57Z","receivedAt":"2015-05-04T19:24:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>> +test_expect_success '--rebase fails with multiple branches' '\n>> +       git reset --hard before-rebase &&\n>> +       test_must_fail git pull --rebase . copy master 2>out &&\n>> +       test_when_finished \"rm -f out\" &&\n>\n> I think it would make sense to switch the previous 2 lines, because the\n> test_when_finished should also be run if the test actually fails.\n\nExcellent.  Thanks for carefully reading patches.\n"},{"id":"260588","messageId":"CACRoPnSJdMmJNQ4xrKr5Og6rZOjCm4Ad8rv5dVfTZod6d5pKWg@mail.gmail.com","threadId":"39223","inReplyTo":"xmqqlhh4tfd0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/7] Improve git-pull test coverage","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-05T10:39:55Z","receivedAt":"2015-05-05T10:39:55Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Tue, May 5, 2015 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I didn't like the grepping of error messages alone as a way to\n> diagnose the failure, though.  When we expect the pull to fail\n> without touching anything in the working tree, I'd prefer to see\n> that tested explicitly (e.g. \"if pull mistakenly tried to go ahead\n> and touch this file that would be involved in the merge, its\n> contents would change---let's make sure that does not happen by\n> making sure the contents before and after are the same\" kind of\n> thing).\n\nYes, you're right. Some tests do need these additional checks. Will\nadd them in the re-roll.\n\nJust to be sure, aside from checking that git-pull does not touch\nfiles it should not touch in the event of an error, I do believe we\nalso need to check its error message, as printing out the correct\nuser-friendly error messages is one of the features of git-pull.\n\nRegards,\nPaul\n"},{"id":"260599","messageId":"CACRoPnSeJT8MK5esRManC7a7v4D2jA4d8YOSvW90+rnoDxuhTQ@mail.gmail.com","threadId":"39223","inReplyTo":"CAGZ79kbSMbhW2FLK05CaQjJjSTqEurB8iyeEBgyA1wbWJ4g2vw@mail.gmail.com","subject":"Re: [PATCH 5/7] t5520: test --rebase with multiple branches","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-05T16:00:27Z","receivedAt":"2015-05-05T16:00:27Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Tue, May 5, 2015 at 1:09 AM, Stefan Beller <sbeller@google.com> wrote:\n> On Sat, May 2, 2015 at 8:37 AM, Paul Tan <pyokagan@gmail.com> wrote:\n>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n>> index 25d519d..17c63ff 100755\n>> --- a/t/t5520-pull.sh\n>> +++ b/t/t5520-pull.sh\n>> @@ -223,6 +223,14 @@ test_expect_success '--rebase' '\n>>         test $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n>>         test new = $(git show HEAD:file2)\n>>  '\n>> +\n>> +test_expect_success '--rebase fails with multiple branches' '\n>> +       git reset --hard before-rebase &&\n>> +       test_must_fail git pull --rebase . copy master 2>out &&\n>> +       test_when_finished \"rm -f out\" &&\n>\n> I think it would make sense to switch the previous 2 lines, because the\n> test_when_finished should also be run if the test actually fails.\n> The actual tested part may segfault, which would interrupt the && chain\n> from reaching the test_when_finished line. And here, the line to test\n> is meant to be the \"git pull --rebase\" line, so we'd assume it may be\n> stopping there due to failures in the tested program.\n>\n> If you grep through the code base, you'll find lots of test_when_finished\n> commands as the very first command (or the earliest spot where it makes\n> sense).\n\nHmm, thinking about it again, I actually don't really think there's\nthat much of a need to \"rm -f out\" in this case ;-)\n\nBut yes, I should go review the placements of the test_when_finished calls :-).\n\nThanks.\n"},{"id":"260646","messageId":"CACRoPnQ5EaCmO-tmDxmNW_snFLbFNOq06wdF_p4FPHHv+HiJBA@mail.gmail.com","threadId":"39223","inReplyTo":"vpqvbg8u5sq.fsf@anie.imag.fr","subject":"Re: [PATCH 2/7] t5520: implement tests for no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-06T06:04:25Z","receivedAt":"2015-05-06T06:04:25Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Mon, May 4, 2015 at 4:04 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>> I'm having trouble hitting the 1st case without resorting to the hack below. A\n>> detached HEAD will always have no remote configured, and the code flow would\n>> make it such that case (4) is hit in the detached HEAD case instead of case\n>> (1).\n>\n> This should appear in comments in the test 'fail if not on a branch'.\n> People reading your [branch \"\"] in the future won't look for\n> below-triple-dash comments in the mailing-list archives ...\n>\n> And actually, it would be more user-friendly to trigger this error\n> message in the normal senario, i.e. check for 1. before 4. in the code.\n> This was most likely the intension of the programmer who wrote this\n> error message. You may want to fix this now, or add a\n> test_expect_failure which will become a test_expect_success when you\n> replace git-pull.sh with builtin/pull.c.\n\nAh, I just figured out how to trigger this error. Just for reference,\nbefore I forget:\n\n1. HEAD must be detached (such that there is no configured remote)\n\n2. The default remote \"origin\" must exist, so that git-fetch will\nsucceed even though there is no configured remote for the detached\nHEAD.\n\n3. git pull must be called with no arguments, to avoid (4).\n\nI guess everything is working as intended. I copied the different\ncases from the commit message of a8c9bef4, but I think that they\nshould be re-ordered to match the logic in the code. Furthermore,\ncases (1) and (2) could probably be explained as:\n\n(1) git-fetch succeeded in fetching from the branch's or repo's\ndefault remote, but:\n\n(1a) We are not on a branch, so there will never be a configured\nupstream branch to merge with.\n\n(1b) We are on a branch, but there is no configured upstream branch to\nmerge with.\n"},{"id":"260647","messageId":"CACRoPnQMAsy3=VoOkjF61QujPMa0L=WmMXqLHakcoyjxaSRx6g@mail.gmail.com","threadId":"39223","inReplyTo":"CACRoPnQ5EaCmO-tmDxmNW_snFLbFNOq06wdF_p4FPHHv+HiJBA@mail.gmail.com","subject":"Re: [PATCH 2/7] t5520: implement tests for no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-06T06:06:47Z","receivedAt":"2015-05-06T06:06:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, May 6, 2015 at 2:04 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> Ah, I just figured out how to trigger this error. Just for reference,\n> before I forget:\n\nWhoops, hit send too early.\n\nThe test for case (1) would look something like:\n\ntest_expect_success 'fail if not on a branch' '\n    git remote add origin . &&\n    test_when_finished \"git remote rm origin\" &&\n    git checkout HEAD^{} &&\n    test_when_finished \"git checkout -f copy\" &&\n    test_must_fail git pull 2>out &&\n    test_i18ngrep \"You are not currently on a branch\" out\n'\n"},{"id":"260649","messageId":"CACRoPnRKcDcvRgMqQefXHhaoFs6s3UMb3CNvNOxo-Rt8hyYnFg@mail.gmail.com","threadId":"39223","inReplyTo":"xmqqlhh4tfd0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/7] Improve git-pull test coverage","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-06T06:30:44Z","receivedAt":"2015-05-06T06:30:44Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Tue, May 5, 2015 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> One test was duplicated with jc/merge series and caused a textual\n> conflict, but that was nothing major.\n\nI will be rebasing the git-pull tests on top of your jc/merge patch\nseries (as it makes the git-pull rewrite easier), so I will remove\nthat offending patch.\n\nThanks,\nPaul\n"}]}