{"thread":{"id":"39353","subject":"[PATCH v4 0/8] Improve git-pull test coverage","startedAt":"2015-05-18T13:32:50Z","lastAt":"2015-06-05T10:44:41Z","messageCount":24,"participants":["Paul Tan","Johannes Schindelin","Junio C Hamano","Stefan Beller","Jeff King"],"isPatch":true,"patchVersion":4,"patchTotal":8},"messages":[{"id":"261430","messageId":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":null,"subject":"[PATCH v4 0/8] Improve git-pull test coverage","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:50Z","receivedAt":"2015-05-18T13:32:50Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"This is a re-roll of [1]. This series depends on jc/merge.\n\nThis patch series improves test coverage of git-pull.sh, and is part of my\nGSoC project to rewrite git-pull into a builtin. Improving test coverage\nhelps to prevent regressions that could occur due to the rewrite.\n\nThis re-roll contains various small fixups. It also removes the use of\n\"verbose\" for now, since its use is currently under discussion.\n\nThanks Junio and Matthieu for the reviews last round, and thanks Michael for\nreporting the test failure.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268950\n\nPaul Tan (8):\n  t5520: prevent field splitting in content comparisons\n  t5520: test 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  t5520: check reflog action in fast-forward merge\n\n t/t5520-pull.sh         | 199 +++++++++++++++++++++++++++++++++++++++---------\n t/t5521-pull-options.sh |  13 ++++\n 2 files changed, 176 insertions(+), 36 deletions(-)\n\n-- \n2.1.4\n"},{"id":"261431","messageId":"1431955978-17890-2-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 1/8] t5520: prevent field splitting in content comparisons","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:51Z","receivedAt":"2015-05-18T13:32:51Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Many tests in t5520 used the following to test the contents of files:\n\n\ttest `cat file` = expected\n\nor\n\n\ttest $(cat file) = expected\n\nThese 2 forms, however, will be affected by field splitting and,\ndepending on the value of $IFS, may be split into multiple arguments,\nmaking the test fail in mysterious ways.\n\nReplace the above 2 forms with:\n\n\ttest \"$(cat file)\" = expected\n\nas quoting the command substitution will prevent field splitting.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n* Removed use of \"verbose\"\n\n* The use of \"wc -l\" is not quoted, as the output of \"wc -l\" on a Mac contains\n  leading whitespace. See [1].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268950/focus=269052\n\n t/t5520-pull.sh | 70 ++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 35 insertions(+), 35 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 7efd45b..5e4db67 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -93,9 +93,9 @@ test_expect_success 'test . as a remote' '\n \techo updated >file &&\n \tgit commit -a -m updated &&\n \tgit checkout copy &&\n-\ttest `cat file` = file &&\n+\ttest \"$(cat file)\" = file &&\n \tgit pull &&\n-\ttest `cat file` = updated\n+\ttest \"$(cat file)\" = updated\n '\n \n test_expect_success 'the default remote . should not break explicit pull' '\n@@ -104,9 +104,9 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \tgit commit -a -m modified &&\n \tgit checkout copy &&\n \tgit reset --hard HEAD^ &&\n-\ttest `cat file` = file &&\n+\ttest \"$(cat file)\" = file &&\n \tgit pull . second &&\n-\ttest `cat file` = modified\n+\ttest \"$(cat file)\" = modified\n '\n \n test_expect_success '--rebase' '\n@@ -119,23 +119,23 @@ test_expect_success '--rebase' '\n \tgit commit -m \"new file\" &&\n \tgit tag before-rebase &&\n \tgit pull --rebase . copy &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\ttest \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n+\ttest new = \"$(git show HEAD:file2)\"\n '\n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\ttest \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n+\ttest new = \"$(git show HEAD:file2)\"\n '\n \n test_expect_success 'branch.to-rebase.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config branch.to-rebase.rebase true &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\ttest \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n+\ttest new = \"$(git show HEAD:file2)\"\n '\n \n test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n@@ -143,8 +143,8 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n \ttest_config pull.rebase true &&\n \ttest_config branch.to-rebase.rebase false &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^) != $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\ttest \"$(git rev-parse HEAD^)\" != \"$(git rev-parse copy)\" &&\n+\ttest new = \"$(git show HEAD:file2)\"\n '\n \n # add a feature branch, keep-merge, that is merged into master, so the\n@@ -163,33 +163,33 @@ test_expect_success 'pull.rebase=false create a new merge commit' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase false &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\ttest \"$(git rev-parse HEAD^1)\" = \"$(git rev-parse before-preserve-rebase)\" &&\n+\ttest \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse copy)\" &&\n+\ttest file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success 'pull.rebase=true flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\ttest \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\ttest file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase 1 &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\ttest \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\ttest file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+\ttest \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\ttest \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse keep-merge)\"\n '\n \n test_expect_success 'pull.rebase=invalid fails' '\n@@ -202,25 +202,25 @@ test_expect_success '--rebase=false create a new merge commit' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull --rebase=false . copy &&\n-\ttest $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\ttest \"$(git rev-parse HEAD^1)\" = \"$(git rev-parse before-preserve-rebase)\" &&\n+\ttest \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse copy)\" &&\n+\ttest file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success '--rebase=true rebases and flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\n \tgit pull --rebase=true . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\ttest \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\ttest file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success '--rebase=preserve rebases and merges keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull --rebase=preserve . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+\ttest \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\ttest \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse keep-merge)\"\n '\n \n test_expect_success '--rebase=invalid fails' '\n@@ -232,8 +232,8 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\n \tgit pull --rebase . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\ttest \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\ttest file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success '--rebase with rebased upstream' '\n@@ -250,7 +250,7 @@ test_expect_success '--rebase with rebased upstream' '\n \tgit tag to-rebase-orig &&\n \tgit pull --rebase me copy &&\n \ttest \"conflicting modification\" = \"$(cat file)\" &&\n-\ttest file = $(cat file2)\n+\ttest file = \"$(cat file2)\"\n \n '\n \n@@ -261,7 +261,7 @@ test_expect_success '--rebase with rebased default upstream' '\n \tgit reset --hard to-rebase-orig &&\n \tgit pull --rebase &&\n \ttest \"conflicting modification\" = \"$(cat file)\" &&\n-\ttest file = $(cat file2)\n+\ttest file = \"$(cat file2)\"\n \n '\n \n@@ -282,7 +282,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '\n \n \tgit checkout to-rebase &&\n \tgit update-ref refs/remotes/me/copy copy^ &&\n-\tCOPY=$(git rev-parse --verify me/copy) &&\n+\tCOPY=\"$(git rev-parse --verify me/copy)\" &&\n \tgit rebase --onto $COPY copy &&\n \ttest_config branch.to-rebase.remote me &&\n \ttest_config branch.to-rebase.merge refs/heads/copy &&\n@@ -290,10 +290,10 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '\n \techo dirty >> file &&\n \tgit add file &&\n \ttest_must_fail git pull &&\n-\ttest $COPY = $(git rev-parse --verify me/copy) &&\n+\ttest \"$COPY\" = \"$(git rev-parse --verify me/copy)\" &&\n \tgit checkout HEAD -- file &&\n \tgit pull &&\n-\ttest $COPY != $(git rev-parse --verify me/copy)\n+\ttest \"$COPY\" != \"$(git rev-parse --verify me/copy)\"\n \n '\n \n-- \n2.1.4\n"},{"id":"261438","messageId":"1431955978-17890-3-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:52Z","receivedAt":"2015-05-18T13:32:52Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"a8c9bef (pull: improve advice for unconfigured error case, 2009-10-05)\nfully established the current advices given by git-pull for the\ndifferent cases where git-fetch will not have anything marked for merge:\n\n1. We fetched from a specific remote, and a refspec was given, but it\n   ended up not fetching anything. This is usually because the user\n   provided a wildcard refspec which had no matches on the remote end.\n\n2. We fetched from a non-default remote, but didn't specify a branch to\n   merge. We can't use the configured one because it applies to the\n   default remote, and thus the user must specify the branches to merge.\n\n3. We fetched from the branch's or repo's default remote, but:\n\n   a. We are not on a branch, so there will never be a configured branch\n      to merge with.\n\n   b. We are on a branch, but there is no configured branch to merge\n      with.\n\n4. We fetched from the branch's or repo's default remote, but the\n   configured branch to merge didn't get fetched (either it doesn't\n   exist, or wasn't part of the configured fetch refspec)\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\n* Removed use of \"verbose\".\n\n t/t5520-pull.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 55 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 5e4db67..4a2c0a1 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -109,6 +109,61 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \ttest \"$(cat file)\" = modified\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 \"$(cat file)\" = file &&\n+\ttest_must_fail git pull . \"refs/nonexisting1/*:refs/nonexisting2/*\" 2>err &&\n+\ttest_i18ngrep \"no candidates for merging\" err &&\n+\ttest \"$(cat file)\" = file\n+'\n+\n+test_expect_success 'fail if no branches specified with non-default remote' '\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 \"$(cat file)\" = file &&\n+\ttest_config branch.test.remote origin &&\n+\ttest_must_fail git pull test_remote 2>err &&\n+\ttest_i18ngrep \"specify a branch on the command line\" err &&\n+\ttest \"$(cat file)\" = file\n+'\n+\n+test_expect_success 'fail if not on a branch' '\n+\tgit remote add origin . &&\n+\ttest_when_finished \"git remote remove origin\" &&\n+\tgit checkout HEAD^ &&\n+\ttest_when_finished \"git checkout -f copy\" &&\n+\ttest \"$(cat file)\" = file &&\n+\ttest_must_fail git pull 2>err &&\n+\ttest_i18ngrep \"not currently on a branch\" err &&\n+\ttest \"$(cat file)\" = file\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 \"$(cat file)\" = file &&\n+\ttest_must_fail git pull 2>err &&\n+\ttest_i18ngrep \"no tracking information\" err &&\n+\ttest \"$(cat file)\" = file\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 \"$(cat file)\" = file &&\n+\ttest_must_fail git pull 2>err &&\n+\ttest_i18ngrep \"no such ref was fetched\" err &&\n+\ttest \"$(cat file)\" = file\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"261436","messageId":"1431955978-17890-4-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 3/8] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:53Z","receivedAt":"2015-05-18T13:32:53Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit d38a30d (Be more user-friendly when refusing to do something\nbecause of conflict., 2010-01-12) introduced code paths to git-pull\nwhich will error out with user-friendly advices if the user is in the\nmiddle of a merge or 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\n* Use the stricter \"Pull is not possible because you have unmerged files\"\n  instead of you have unmerged files\" as git-merge will also raise a similar\n  error \"merge is not possible because you have unmerged files\"\n\n* Also, grep for \"You have not concluded your merge\" instead of \"have not\n  concluded your merge\" as the latter reads weird to me personally.\n\n t/t5520-pull.sh | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 4a2c0a1..3bc0594 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -164,6 +164,26 @@ test_expect_success 'fail if upstream branch does not exist' '\n \ttest \"$(cat file)\" = file\n '\n \n+test_expect_success 'fail if the index has unresolved entries' '\n+\tgit checkout -b third second^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\ttest \"$(cat file)\" = file &&\n+\techo modified2 >file &&\n+\tgit commit -a -m modified2 &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_must_fail git pull . second &&\n+\ttest -n \"$(git ls-files -u)\" &&\n+\tcp file expected &&\n+\ttest_must_fail git pull . second 2>err &&\n+\ttest_i18ngrep \"Pull is not possible because you have unmerged files\" err &&\n+\ttest_cmp expected file &&\n+\tgit add file &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_must_fail git pull . second 2>err &&\n+\ttest_i18ngrep \"You have not concluded your merge\" err &&\n+\ttest_cmp expected file\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"261432","messageId":"1431955978-17890-5-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 4/8] t5520: test work tree fast-forward when fetch updates head","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:54Z","receivedAt":"2015-05-18T13:32:54Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since b10ac50 (Fix pulling into the same branch., 2005-08-25), 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\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n* Removed use of \"verbose\".\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 3bc0594..3a53a5e 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -184,6 +184,27 @@ test_expect_success 'fail if the index has unresolved entries' '\n \ttest_cmp expected file\n '\n \n+test_expect_success 'fast-forwards working tree if branch head is updated' '\n+\tgit checkout -b third second^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\ttest \"$(cat file)\" = file &&\n+\tgit pull . second:third 2>err &&\n+\ttest_i18ngrep \"fetch updated the current branch head\" err &&\n+\ttest \"$(cat file)\" = modified &&\n+\ttest \"$(git rev-parse third)\" = \"$(git rev-parse second)\"\n+'\n+\n+test_expect_success 'fast-forward fails with conflicting work tree' '\n+\tgit checkout -b third second^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\ttest \"$(cat file)\" = file &&\n+\techo conflict >file &&\n+\ttest_must_fail git pull . second:third 2>err &&\n+\ttest_i18ngrep \"Cannot fast-forward your working tree\" err &&\n+\ttest \"$(cat file)\" = conflict &&\n+\ttest \"$(git rev-parse third)\" = \"$(git rev-parse second)\"\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"261435","messageId":"1431955978-17890-6-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 5/8] t5520: test --rebase with multiple branches","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:55Z","receivedAt":"2015-05-18T13:32:55Z","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 51b2ead (disallow providing multiple upstream branches to rebase,\npull --rebase, 2009-02-18), 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* Removed use of \"verbose\".\n\n t/t5520-pull.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 3a53a5e..f991439 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -218,6 +218,15 @@ 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>err &&\n+\ttest \"$(git rev-parse HEAD)\" = \"$(git rev-parse before-rebase)\" &&\n+\ttest_i18ngrep \"Cannot rebase onto multiple branches\" err &&\n+\ttest modified = \"$(git show HEAD:file)\"\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":"261433","messageId":"1431955978-17890-7-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 6/8] t5520: test --rebase failure on unborn branch with index","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:56Z","receivedAt":"2015-05-18T13:32:56Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit 19a7fcb (allow pull --rebase on branch yet to be born,\n2009-08-11) special cases git-pull on an unborn branch in a different\ncode path such that git-pull --rebase is still valid even though there\nis 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\n* Removed use of \"verbose\".\n\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 f991439..4d649a5 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -413,6 +413,21 @@ 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+\ttest_when_finished \"rm -rf empty_repo2\" &&\n+\tgit init 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 \"$(git ls-files)\" = staged-file &&\n+\t\ttest_must_fail git pull --rebase .. master 2>../err &&\n+\t\ttest \"$(git ls-files)\" = staged-file &&\n+\t\ttest \"$(git show :staged-file)\" = staged-file\n+\t) &&\n+\ttest_i18ngrep \"unborn branch with changes added to the index\" err\n+'\n+\n test_expect_success 'setup for detecting upstreamed changes' '\n \tmkdir src &&\n \t(cd src &&\n-- \n2.1.4\n"},{"id":"261437","messageId":"1431955978-17890-8-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 7/8] t5521: test --dry-run does not make any changes","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:57Z","receivedAt":"2015-05-18T13:32:57Z","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* No changes\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..56e7377 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+\ttest_when_finished \"rm -rf clonedry\" &&\n+\tgit init 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":"261434","messageId":"1431955978-17890-9-git-send-email-pyokagan@gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v4 8/8] t5520: check reflog action in fast-forward merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:32:58Z","receivedAt":"2015-05-18T13:32:58Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"When testing a fast-forward merge with git-pull, check to see if the\nreflog action is \"pull\" with the arguments passed to git-pull.\n\nWhile we are in the vicinity, remove the empty line as well.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n* Removed use of \"verbose\".\n\n t/t5520-pull.sh | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 4d649a5..62dbfb5 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -86,7 +86,6 @@ test_expect_success 'pulling into void must not create an octopus' '\n '\n \n test_expect_success 'test . as a remote' '\n-\n \tgit branch copy master &&\n \tgit config branch.copy.remote . &&\n \tgit config branch.copy.merge refs/heads/master &&\n@@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '\n \tgit checkout copy &&\n \ttest \"$(cat file)\" = file &&\n \tgit pull &&\n-\ttest \"$(cat file)\" = updated\n+\ttest \"$(cat file)\" = updated &&\n+\tgit reflog -1 >reflog.actual &&\n+\tsed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n+\techo \"OBJID HEAD@{0}: pull: Fast-forward\" >reflog.expected &&\n+\ttest_cmp reflog.expected reflog.fuzzy\n '\n \n test_expect_success 'the default remote . should not break explicit pull' '\n@@ -106,7 +109,11 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \tgit reset --hard HEAD^ &&\n \ttest \"$(cat file)\" = file &&\n \tgit pull . second &&\n-\ttest \"$(cat file)\" = modified\n+\ttest \"$(cat file)\" = modified &&\n+\tgit reflog -1 >reflog.actual &&\n+\tsed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n+\techo \"OBJID HEAD@{0}: pull . second: Fast-forward\" >reflog.expected &&\n+\ttest_cmp reflog.expected reflog.fuzzy\n '\n \n test_expect_success 'fail if wildcard spec does not match any refs' '\n-- \n2.1.4\n"},{"id":"261468","messageId":"e89b333476cbb1e546371a07b357cd42@www.dscho.org","threadId":"39353","inReplyTo":"1431955978-17890-3-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-18T15:08:38Z","receivedAt":"2015-05-18T15:08:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-18 15:32, Paul Tan wrote:\n\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 5e4db67..4a2c0a1 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -109,6 +109,61 @@ test_expect_success 'the default remote . should\n> not break explicit pull' '\n>  \ttest \"$(cat file)\" = modified\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\nWhen I read this line, I immediately asked myself whether the branch would be deleted even if the test case failed. I then tested this theory by editing the first test case (\"setup\") like this:\n\n-- snip --\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 227d293..3adc702 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -15,9 +15,12 @@ test_expect_success setup '\n \n \techo file >file &&\n \tgit add file &&\n+test_when_finished \"rm file\" &&\n+false &&\n \tgit commit -a -m original\n \n '\n+exit 1\n \n test_expect_success 'pulling into void' '\n \tmkdir cloned &&\n-- snap --\n\nand indeed, the file \"file\" was gone, even if the test case failed. I therefore believe that this \"test_when_finished\" cleanup might make debugging substantially harder. Maybe we can drop these lines from this patch?\n\nCiao,\nDscho\n"},{"id":"261469","messageId":"d3f29ef8736ae4c077b754ec6e410308@www.dscho.org","threadId":"39353","inReplyTo":"1431955978-17890-4-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v4 3/8] t5520: test for failure if index has unresolved entries","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-18T15:13:19Z","receivedAt":"2015-05-18T15:13:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-18 15:32, Paul Tan wrote:\n\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 4a2c0a1..3bc0594 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -164,6 +164,26 @@ test_expect_success 'fail if upstream branch does\n> not exist' '\n>  \ttest \"$(cat file)\" = file\n>  '\n>  \n> +test_expect_success 'fail if the index has unresolved entries' '\n> +\tgit checkout -b third second^ &&\n> +\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n\nIf you agree with my argument in 2/8, it would be nice to drop this line, too.\n\n> +\ttest \"$(cat file)\" = file &&\n> +\techo modified2 >file &&\n> +\tgit commit -a -m modified2 &&\n\nThese two lines could be combined into \"test_commit modified2 file\".\n\nCiao,\nDscho\n"},{"id":"261471","messageId":"8e51ff8168d050cb2f8fedd4464b2f7b@www.dscho.org","threadId":"39353","inReplyTo":"1431955978-17890-9-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v4 8/8] t5520: check reflog action in fast-forward merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-18T15:20:32Z","receivedAt":"2015-05-18T15:20:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-18 15:32, Paul Tan wrote:\n\n> @@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '\n>  \tgit checkout copy &&\n>  \ttest \"$(cat file)\" = file &&\n>  \tgit pull &&\n> -\ttest \"$(cat file)\" = updated\n> +\ttest \"$(cat file)\" = updated &&\n> +\tgit reflog -1 >reflog.actual &&\n> +\tsed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n\nActually, let's use \"s/^[0-9a-f]*/OBJID/\" instead: you only want to replace the first few characters.\n\n> @@ -106,7 +109,11 @@ test_expect_success 'the default remote . should\n> not break explicit pull' '\n>  \tgit reset --hard HEAD^ &&\n>  \ttest \"$(cat file)\" = file &&\n>  \tgit pull . second &&\n> -\ttest \"$(cat file)\" = modified\n> +\ttest \"$(cat file)\" = modified &&\n> +\tgit reflog -1 >reflog.actual &&\n> +\tsed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n\nSame here.\n\nCiao,\nDscho\n"},{"id":"261472","messageId":"621e954c6904ca5af59cbe4158035788@www.dscho.org","threadId":"39353","inReplyTo":"1431955978-17890-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v4 4/8] t5520: test work tree fast-forward when fetch updates head","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-18T15:22:29Z","receivedAt":"2015-05-18T15:22:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-18 15:32, Paul Tan wrote:\n\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 3bc0594..3a53a5e 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -184,6 +184,27 @@ test_expect_success 'fail if the index has\n> unresolved entries' '\n>  \ttest_cmp expected file\n>  '\n>  \n> +test_expect_success 'fast-forwards working tree if branch head is updated' '\n> +\tgit checkout -b third second^ &&\n> +\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n\nIf you follow my argument in 2/8, this line, and...\n\n> [...]\n> +test_expect_success 'fast-forward fails with conflicting work tree' '\n> +\tgit checkout -b third second^ &&\n> +\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n\n... this line should be dropped, too.\n\nThis comment concludes my review of v4 of this patch series. Well done!\n\nCiao,\nDscho\n"},{"id":"261477","messageId":"xmqqoalhdbhx.fsf@gitster.dls.corp.google.com","threadId":"39353","inReplyTo":"e89b333476cbb1e546371a07b357cd42@www.dscho.org","subject":"Re: [PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-18T17:46:50Z","receivedAt":"2015-05-18T17:46:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\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>\n> When I read this line, I immediately asked myself whether the\n> branch would be deleted even if the test case failed. I then\n> tested this theory by editing the first test case (\"setup\") like\n> this:\n> ...\n> and indeed, the file \"file\" was gone, even if the test case\n> failed. I therefore believe that this \"test_when_finished\" cleanup\n> might make debugging substantially harder. Maybe we can drop these\n> lines from this patch?\n\nThe test framework is aware of the fact that it needs to help the\npeople who are debugging the scripts.  The support is limited to the\ncase in which you run it under the -i option, i.e.\n\n\t$ cd t\n        $ sh ./t5520-pull.sh -i -v\n\nwill refrain from running test_when_finished scripts when the test\npiece fails.  Even though this is only limited to -i, I found it\noften sufficient for debugging.\n"},{"id":"261482","messageId":"CAGZ79kbXcWiK-9_NDCERNbi-m1cPiuukZ75r6TWwoUBsnyUGiA@mail.gmail.com","threadId":"39353","inReplyTo":"1431955978-17890-7-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v4 6/8] t5520: test --rebase failure on unborn branch with index","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-18T18:00:01Z","receivedAt":"2015-05-18T18:00:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, May 18, 2015 at 6:32 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> Commit 19a7fcb (allow pull --rebase on branch yet to be born,\n> 2009-08-11) special cases git-pull on an unborn branch in a different\n> code path such that git-pull --rebase is still valid even though there\n> is no HEAD yet.\n>\n> This code path still ensures that there is no index in order not to lose\n> any staged changes. Implement a test to ensure that this check is\n> triggered.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>\n> * Removed use of \"verbose\".\n>\n>\n>  t/t5520-pull.sh | 15 +++++++++++++++\n>  1 file changed, 15 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index f991439..4d649a5 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -413,6 +413,21 @@ test_expect_success 'pull --rebase works on branch yet to be born' '\n>         test_cmp expect actual\n>  '\n>\n> +test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n> +       test_when_finished \"rm -rf empty_repo2\" &&\n> +       git init empty_repo2 &&\n> +       (\n> +               cd empty_repo2 &&\n> +               echo staged-file >staged-file &&\n> +               git add staged-file &&\n> +               test \"$(git ls-files)\" = staged-file &&\n> +               test_must_fail git pull --rebase .. master 2>../err &&\n> +               test \"$(git ls-files)\" = staged-file &&\n> +               test \"$(git show :staged-file)\" = staged-file\n> +       ) &&\n> +       test_i18ngrep \"unborn branch with changes added to the index\" err\n\nSo when seeing this line outside the parenthesis section, I immediately thought\nthere must be a reason you put it outside. The reason is not obvious\nto me though.\nSo I'd suggest to move the test_i18ngrep inside the section above.\n\n> +'\n> +\n>  test_expect_success 'setup for detecting upstreamed changes' '\n>         mkdir src &&\n>         (cd src &&\n> --\n> 2.1.4\n>\n"},{"id":"261483","messageId":"xmqqa8x1dajl.fsf@gitster.dls.corp.google.com","threadId":"39353","inReplyTo":"1431955978-17890-2-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v4 1/8] t5520: prevent field splitting in content comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-18T18:07:26Z","receivedAt":"2015-05-18T18:07:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Many tests in t5520 used the following to test the contents of files:\n>\n> \ttest `cat file` = expected\n>\n> or\n>\n> \ttest $(cat file) = expected\n>\n> These 2 forms, however, will be affected by field splitting and,\n> depending on the value of $IFS, may be split into multiple arguments,\n> making the test fail in mysterious ways.\n>\n> Replace the above 2 forms with:\n>\n> \ttest \"$(cat file)\" = expected\n>\n> as quoting the command substitution will prevent field splitting.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n> * Removed use of \"verbose\"\n>\n> * The use of \"wc -l\" is not quoted, as the output of \"wc -l\" on a Mac contains\n>   leading whitespace. See [1].\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/268950/focus=269052\n>\n>  t/t5520-pull.sh | 70 ++++++++++++++++++++++++++++-----------------------------\n>  1 file changed, 35 insertions(+), 35 deletions(-)\n\nOverall the series was a pleasant read.\n\nSome common patterns (my mind is still continuing the verbose_test\ntopic) I noticed were\n\n (1) comparing output from two rev-parse output for object names. We\n     have test_cmp_rev we can use for better readability (but its\n     implementaiton may want to be updated not to contaminate the\n     working tree unnecessarily). e.g\n\n\ttest \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\"\n     => test_cmp_rev HEAD^ copy\n \n (2) checking contents of a file, either from a working tree, from\n     the index or from a rev, with a fixed short string.\n\n\ttest_contents () {\n                case \"$1\" in\n                *:*)\tgit cat-file blob \"$1\" ;;\n                *)\tcat \"$1\" ;;\n                esac >actual &&\n\t\tprintf \"%s\" \"$2\" >expect &&\n\t\tif ! cmp -s expect actual\n                then\n\t\t\techo \"Unexpected contents in '$1'\"\n\t\t\ttest_cmp expect actual\n\t\telse\n\t\t\t: good ;\n\t\tfi\n\t}\n\n     or something.\n\n\nI think it is a good idea *not* to address these patterns within\nthis series, and have people come back to them _after_ the series\nsettles, to reduce code churn and unnecessary conflicts.\n\nThanks.\n"},{"id":"261492","messageId":"20150518185554.GB11463@peff.net","threadId":"39353","inReplyTo":"xmqqoalhdbhx.fsf@gitster.dls.corp.google.com","subject":"debugging git tests, was: Re: [PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-05-18T18:55:54Z","receivedAt":"2015-05-18T18:55:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 18, 2015 at 10:46:50AM -0700, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\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> >\n> > When I read this line, I immediately asked myself whether the\n> > branch would be deleted even if the test case failed. I then\n> > tested this theory by editing the first test case (\"setup\") like\n> > this:\n> > ...\n> > and indeed, the file \"file\" was gone, even if the test case\n> > failed. I therefore believe that this \"test_when_finished\" cleanup\n> > might make debugging substantially harder. Maybe we can drop these\n> > lines from this patch?\n> \n> The test framework is aware of the fact that it needs to help the\n> people who are debugging the scripts.  The support is limited to the\n> case in which you run it under the -i option, i.e.\n> \n> \t$ cd t\n>         $ sh ./t5520-pull.sh -i -v\n> \n> will refrain from running test_when_finished scripts when the test\n> piece fails.  Even though this is only limited to -i, I found it\n> often sufficient for debugging.\n\nIf you don't use \"-i\", you are pretty much screwed anyway, because the\nsubsequent tests will stomp all over the state of the test directory.\nMany a head-scratching session has been caused by looking at the wrong\nstate, and these days my go-to options for debugging a test are \"-v -i\".\nBut since we are talking about it in a related thread, I will advertise\nthe new \"-x\" here, too.  :)\n\nAs a side note, I've also considered better support for running the\ndebugger on git commands inside a test (right now, I usually stick a\n\"gdb --args\" in the pipeline, but you have to remember to run with \"-v\",\nand to redirect stdin appropriately). Do other people have this\nannoyance, too?\n\nI'm vaguely thinking of something like putting debug support into\nbin-wrappers/git, but activating it only for certain tests (so you could\nsay \"t5520-pull.sh --gdb=10\", and git would start under the debugger\nonly for test 10). I think we'd also have to use gdbserver for I/O\nsanity, and maybe provide short script to do:\n\n   gdb -ex \"target remote localhost:$some_port\" \"$TEST_DIRECTORY\"/../git\n\nThat still doesn't cover all cases (when git spawns an external command,\nyou probably want to run the debugger on that; likewise, I have a\ngit-remote-debug hack for debugging remote-curl). I suspect with clever\nuse of gdb options that you could convince the original gdb invocation\nto end up tracing the process you care about, though.\n\n-Peff\n"},{"id":"261507","messageId":"xmqqd21xbrw6.fsf@gitster.dls.corp.google.com","threadId":"39353","inReplyTo":"20150518185554.GB11463@peff.net","subject":"Re: debugging git tests, was: Re: [PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-18T19:35:37Z","receivedAt":"2015-05-18T19:35:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, May 18, 2015 at 10:46:50AM -0700, Junio C Hamano wrote:\n>\n>> The test framework is aware of the fact that it needs to help the\n>> people who are debugging the scripts.  The support is limited to the\n>> case in which you run it under the -i option, i.e.\n>> \n>> \t$ cd t\n>>         $ sh ./t5520-pull.sh -i -v\n>> \n>> will refrain from running test_when_finished scripts when the test\n>> piece fails.  Even though this is only limited to -i, I found it\n>> often sufficient for debugging.\n>\n> If you don't use \"-i\", you are pretty much screwed anyway, because the\n> subsequent tests will stomp all over the state of the test directory.\n\nYeah.\n\n> Many a head-scratching session has been caused by looking at the wrong\n> state, and these days my go-to options for debugging a test are \"-v -i\".\n> But since we are talking about it in a related thread, I will advertise\n> the new \"-x\" here, too.  :)\n\nYes, thanks for \"-x\".  That has been very helpful.\n\n> As a side note, I've also considered better support for running the\n> debugger on git commands inside a test (right now, I usually stick a\n> \"gdb --args\" in the pipeline, but you have to remember to run with \"-v\",\n> and to redirect stdin appropriately). Do other people have this\n> annoyance, too?\n\nI usually tweak the script and have it stop before the offending\ntest, and then go through the steps in the test manually X-<.  If it\ncan be more automated, that would be great.\n\nI haven't been ambitious enough to even attempt it so do not have\nanything to add to the implementation ideas at this point.\n\n> I'm vaguely thinking of something like putting debug support into\n> bin-wrappers/git, but activating it only for certain tests (so you could\n> say \"t5520-pull.sh --gdb=10\", and git would start under the debugger\n> only for test 10). I think we'd also have to use gdbserver for I/O\n> sanity, and maybe provide short script to do:\n>\n>    gdb -ex \"target remote localhost:$some_port\" \"$TEST_DIRECTORY\"/../git\n>\n> That still doesn't cover all cases (when git spawns an external command,\n> you probably want to run the debugger on that; likewise, I have a\n> git-remote-debug hack for debugging remote-curl). I suspect with clever\n> use of gdb options that you could convince the original gdb invocation\n> to end up tracing the process you care about, though.\n>\n> -Peff\n"},{"id":"261547","messageId":"1254ff617c50077dc0c08321f168c4ce@www.dscho.org","threadId":"39353","inReplyTo":"20150518185554.GB11463@peff.net","subject":"Re: debugging git tests, was: Re: [PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-19T13:29:23Z","receivedAt":"2015-05-19T13:29:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn 2015-05-18 20:55, Jeff King wrote:\n\n> As a side note, I've also considered better support for running the\n> debugger on git commands inside a test (right now, I usually stick a\n> \"gdb --args\" in the pipeline, but you have to remember to run with \"-v\",\n> and to redirect stdin appropriately). Do other people have this\n> annoyance, too?\n\nYes, I have that annoyance, and I usually want to debug one very specific Git call in that script. Therefore, I made this patch:\n\nhttps://github.com/git-for-windows/git/commit/0f50fd723beb5d27d2d799ef3a0cfc0b0bbd80a8\n\nIt really served me well during the past four months of Git for Windows 2.x work. Of course I also had to avoid the redirection:\n\nhttps://github.com/git-for-windows/git/commit/d27cb6e0572475582677724545a71755d171ea76\n\nSo what I do these days is to run the failing test with `TEST_NO_REDIRECT=1` and with the Git call I want to debug prefixed with `TEST_GDB_GIT=1`.\n\nCiao,\nDscho\n"},{"id":"261759","messageId":"CACRoPnRHAA7w8iJ-GQGPt11Hnm29o6wgP5bEG_q531XmcK-P1Q@mail.gmail.com","threadId":"39353","inReplyTo":"8e51ff8168d050cb2f8fedd4464b2f7b@www.dscho.org","subject":"Re: [PATCH v4 8/8] t5520: check reflog action in fast-forward merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-21T08:07:05Z","receivedAt":"2015-05-21T08:07:05Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Mon, May 18, 2015 at 11:20 PM, Johannes Schindelin\n<johannes.schindelin@gmx.de> wrote:\n> Hi Paul,\n>\n> On 2015-05-18 15:32, Paul Tan wrote:\n>\n>> @@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '\n>>       git checkout copy &&\n>>       test \"$(cat file)\" = file &&\n>>       git pull &&\n>> -     test \"$(cat file)\" = updated\n>> +     test \"$(cat file)\" = updated &&\n>> +     git reflog -1 >reflog.actual &&\n>> +     sed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n>\n> Actually, let's use \"s/^[0-9a-f]*/OBJID/\" instead: you only want to replace the first few characters.\n\nDid you mean \"s/^$_x05[0-9a-f]*/OBJID/\"? (with \"$_x05\" expanding to\n'[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' from test-lib.sh). If not,\nthen it would match even if there was no SHA1 hash.\n\nBut yes, without the \"^\" there will very likely be false positives.\nThanks for catching.\n\n>> @@ -106,7 +109,11 @@ test_expect_success 'the default remote . should\n>> not break explicit pull' '\n>>       git reset --hard HEAD^ &&\n>>       test \"$(cat file)\" = file &&\n>>       git pull . second &&\n>> -     test \"$(cat file)\" = modified\n>> +     test \"$(cat file)\" = modified &&\n>> +     git reflog -1 >reflog.actual &&\n>> +     sed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n>\n> Same here.\n\nRegards,\nPaul\n"},{"id":"261761","messageId":"CACRoPnT8qLnjW2BAsOeqWQUPZPSmLgXOtnyVZg0Pn-ijweowZA@mail.gmail.com","threadId":"39353","inReplyTo":"d3f29ef8736ae4c077b754ec6e410308@www.dscho.org","subject":"Re: [PATCH v4 3/8] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-21T08:15:24Z","receivedAt":"2015-05-21T08:15:24Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Mon, May 18, 2015 at 11:13 PM, Johannes Schindelin\n<johannes.schindelin@gmx.de> wrote:\n> Hi Paul,\n>\n> On 2015-05-18 15:32, Paul Tan wrote:\n>\n>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n>> index 4a2c0a1..3bc0594 100755\n>> --- a/t/t5520-pull.sh\n>> +++ b/t/t5520-pull.sh\n>> @@ -164,6 +164,26 @@ test_expect_success 'fail if upstream branch does\n>> not exist' '\n>>       test \"$(cat file)\" = file\n>>  '\n>>\n>> +test_expect_success 'fail if the index has unresolved entries' '\n>> +     git checkout -b third second^ &&\n>> +     test_when_finished \"git checkout -f copy && git branch -D third\" &&\n>\n> If you agree with my argument in 2/8, it would be nice to drop this line, too.\n\nAs mentioned by Junio in 2/8 the cleanup functions will not run under\n--immediate mode.\n\nBesides, the use of test_when_finished is to send a clear signal that\nthe \"third\" branch is temporary and is not meant to be used by other\ntest cases. Furthermore, subsequent tests assume that the current\nbranch is \"copy\", so it's best to preserve that.\n\n>> +     test \"$(cat file)\" = file &&\n>> +     echo modified2 >file &&\n>> +     git commit -a -m modified2 &&\n>\n> These two lines could be combined into \"test_commit modified2 file\".\n\nFixed, thanks.\n\nRegards,\nPaul\n"},{"id":"261766","messageId":"CACRoPnSuiN-Q0PY97bEuS5GV2pTqmLoSz-1wk2OdTo6aNsGw0A@mail.gmail.com","threadId":"39353","inReplyTo":"CAGZ79kbXcWiK-9_NDCERNbi-m1cPiuukZ75r6TWwoUBsnyUGiA@mail.gmail.com","subject":"Re: [PATCH v4 6/8] t5520: test --rebase failure on unborn branch with index","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-21T08:51:16Z","receivedAt":"2015-05-21T08:51:16Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Tue, May 19, 2015 at 2:00 AM, Stefan Beller <sbeller@google.com> wrote:\n> On Mon, May 18, 2015 at 6:32 AM, Paul Tan <pyokagan@gmail.com> wrote:\n>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n>> index f991439..4d649a5 100755\n>> --- a/t/t5520-pull.sh\n>> +++ b/t/t5520-pull.sh\n>> @@ -413,6 +413,21 @@ test_expect_success 'pull --rebase works on branch yet to be born' '\n>>         test_cmp expect actual\n>>  '\n>>\n>> +test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n>> +       test_when_finished \"rm -rf empty_repo2\" &&\n>> +       git init empty_repo2 &&\n>> +       (\n>> +               cd empty_repo2 &&\n>> +               echo staged-file >staged-file &&\n>> +               git add staged-file &&\n>> +               test \"$(git ls-files)\" = staged-file &&\n>> +               test_must_fail git pull --rebase .. master 2>../err &&\n>> +               test \"$(git ls-files)\" = staged-file &&\n>> +               test \"$(git show :staged-file)\" = staged-file\n>> +       ) &&\n>> +       test_i18ngrep \"unborn branch with changes added to the index\" err\n>\n> So when seeing this line outside the parenthesis section, I immediately thought\n> there must be a reason you put it outside. The reason is not obvious\n> to me though.\n> So I'd suggest to move the test_i18ngrep inside the section above.\n\nRight. Fixed.\n\nRegards,\nPaul\n"},{"id":"261811","messageId":"xmqqk2w1kffa.fsf@gitster.dls.corp.google.com","threadId":"39353","inReplyTo":"CACRoPnRHAA7w8iJ-GQGPt11Hnm29o6wgP5bEG_q531XmcK-P1Q@mail.gmail.com","subject":"Re: [PATCH v4 8/8] t5520: check reflog action in fast-forward merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-21T17:29:13Z","receivedAt":"2015-05-21T17:29:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> On Mon, May 18, 2015 at 11:20 PM, Johannes Schindelin\n> ...\n>>> +     sed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n>>\n>> Actually, let's use \"s/^[0-9a-f]*/OBJID/\" instead: you only want to\n>> replace the first few characters.\n>\n> Did you mean \"s/^$_x05[0-9a-f]*/OBJID/\"? (with \"$_x05\" expanding to\n> '[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' from test-lib.sh). If not,\n> then it would match even if there was no SHA1 hash.\n>\n> But yes, without the \"^\" there will very likely be false positives.\n> Thanks for catching.\n\nI think the suggestion was more about \"do we guarantee that there\nwould always be at least five?\"  It might happen to be the case with\nour current default, but these tests do not fundamentally rely on\nthat default staying the same.  s/^[0-9a-f][0-9a-f]*/OBJECTNAME/g is\nprobably the right balance between cautiousness (you do not want to\nmatch an empty string) and future-proofing (you do not want to rely\non us having at least five).\n\nThanks.\n"},{"id":"263004","messageId":"20150605104441.GA19840@peff.net","threadId":"39353","inReplyTo":"1254ff617c50077dc0c08321f168c4ce@www.dscho.org","subject":"Re: debugging git tests, was: Re: [PATCH v4 2/8] t5520: test no merge candidates cases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-05T10:44:41Z","receivedAt":"2015-06-05T10:44:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 19, 2015 at 03:29:23PM +0200, Johannes Schindelin wrote:\n\n> On 2015-05-18 20:55, Jeff King wrote:\n> \n> > As a side note, I've also considered better support for running the\n> > debugger on git commands inside a test (right now, I usually stick a\n> > \"gdb --args\" in the pipeline, but you have to remember to run with \"-v\",\n> > and to redirect stdin appropriately). Do other people have this\n> > annoyance, too?\n> \n> Yes, I have that annoyance, and I usually want to debug one very specific Git call in that script. Therefore, I made this patch:\n> \n> https://github.com/git-for-windows/git/commit/0f50fd723beb5d27d2d799ef3a0cfc0b0bbd80a8\n> \n> It really served me well during the past four months of Git for Windows 2.x work. Of course I also had to avoid the redirection:\n> \n> https://github.com/git-for-windows/git/commit/d27cb6e0572475582677724545a71755d171ea76\n> \n> So what I do these days is to run the failing test with `TEST_NO_REDIRECT=1` and with the Git call I want to debug prefixed with `TEST_GDB_GIT=1`.\n\nThanks for sharing this (I stuck it on my \"to read\" list and didn't get\nto it until today).\n\nThis is definitely the direction I'd like to go in. I think it would be\ncool to add in a test-harness option enable TEST_GIT_GDB for a specific\ntest. So something like:\n\n  ./t1234-foo.sh --gdb=8\n\nto just enable it for a particular test (of course some test snippets\nrun multiple git invocations, but in practice I think it is few enough\nthat you could just ask gdb to run to completion until you hit the\ninvocation you want).\n\nI don't have any code to share yet. I'm still at the point of collecting\nideas like this, which I'll rage-implement next time I have to do a lot\nof debugging. :)\n\n-Peff\n"}]}