{"thread":{"id":"46123","subject":"[PATCH] t3200: add test for single parameter passed to -m option","startedAt":"2017-06-05T22:38:34Z","lastAt":"2017-06-13T10:47:28Z","messageCount":6,"participants":["Sahil Dua","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"321571","messageId":"0102015c7a68c647-372a5a13-5412-4838-921d-66980bac4099-000000@eu-west-1.amazonses.com","threadId":"46123","inReplyTo":null,"subject":"[PATCH] t3200: add test for single parameter passed to -m option","fromName":"Sahil Dua","fromEmail":"sahildua2305@gmail.com","sentAt":"2017-06-05T22:38:25Z","receivedAt":"2017-06-05T22:38:34Z","isPatch":true,"sender":{"key":"sahildua2305@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5206277?v=4"},"body":"Adds a test for the case when only one parameter is passed to '-m'\n(move/rename) option.\n\nFor example - if 'git branch -m bbb' is run, it should rename the\ncurrently checked out branch to bbb. There was no test for this\nparticular case with only one parameter for -m option. However,\nthere's one similar test case for -M option.\n\nSigned-off-by: Sahil Dua <sahildua2305@gmail.com>\n---\n t/t3200-branch.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex fe62e7c775da6..7504f14bc52f8 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -100,6 +100,14 @@ test_expect_success 'git branch -m n/n n should work' '\n \tgit reflog exists refs/heads/n\n '\n \n+test_expect_success 'git branch -m bbb should rename checked out branch' '\n+\ttest_when_finished git branch -d bbb &&\n+\ttest_when_finished git checkout master &&\n+\tgit checkout -b aaa &&\n+\tgit branch -m bbb &&\n+\tgit reflog exists refs/heads/bbb\n+'\n+\n test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n \tgit branch o/o &&\n \tgit branch o/p &&\n\n--\nhttps://github.com/git/git/pull/371\n"},{"id":"321589","messageId":"xmqqzidl9a8l.fsf@gitster.mtv.corp.google.com","threadId":"46123","inReplyTo":"0102015c7a68c647-372a5a13-5412-4838-921d-66980bac4099-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] t3200: add test for single parameter passed to -m option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-06T01:27:06Z","receivedAt":"2017-06-06T01:27:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sahil Dua <sahildua2305@gmail.com> writes:\n\n> Adds a test for the case when only one parameter is passed to '-m'\n> (move/rename) option.\n>\n> For example - if 'git branch -m bbb' is run, it should rename the\n> currently checked out branch to bbb. There was no test for this\n> particular case with only one parameter for -m option. However,\n> there's one similar test case for -M option.\n>\n> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>\n> ---\n>  t/t3200-branch.sh | 8 ++++++++\n>  1 file changed, 8 insertions(+)\n>\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index fe62e7c775da6..7504f14bc52f8 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -100,6 +100,14 @@ test_expect_success 'git branch -m n/n n should work' '\n>  \tgit reflog exists refs/heads/n\n>  '\n>  \n> +test_expect_success 'git branch -m bbb should rename checked out branch' '\n> +\ttest_when_finished git branch -d bbb &&\n> +\ttest_when_finished git checkout master &&\n> +\tgit checkout -b aaa &&\n> +\tgit branch -m bbb &&\n> +\tgit reflog exists refs/heads/bbb\n> +'\n\nLooks almost good.  Don't we want to also make sure that HEAD points\nat bbb branch, e.g.\n\n\tgit symbolic-ref HEAD >actual &&\n\techo refs/heads/bbb >expect &&\n\ttest_cmp expect actual &&\n\nafter the \"do we have reflog for the bbb branch?\" test?\n\nAlso, I suspect that we care about the reflog that is moved to 'bbb'\nretains entries created for 'aaa'.  So perhaps \"reflog exists\" needs\nto be replaced by something like\n\n\tgit checkout -b aaa &&\n\tgit commit --allow-empty -m \"a new commit\" &&\n\tgit rev-parse aaa@{1} >expect &&\n\tgit branch -m bbb &&\n\t# the topmost entry is about branch creation\n\tgit rev-parse bbb@{2} >actual\n\nno?\n\n>  test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n>  \tgit branch o/o &&\n>  \tgit branch o/p &&\n>\n> --\n> https://github.com/git/git/pull/371\n"},{"id":"321603","messageId":"CALiud+mvAuDhcLr1DSxAxaf+-NtotBBSxgPM6tMJNueNdR8eAQ@mail.gmail.com","threadId":"46123","inReplyTo":"xmqqzidl9a8l.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t3200: add test for single parameter passed to -m option","fromName":"Sahil Dua","fromEmail":"sahildua2305@gmail.com","sentAt":"2017-06-06T08:51:08Z","receivedAt":"2017-06-06T08:51:34Z","isPatch":true,"sender":{"key":"sahildua2305@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5206277?v=4"},"body":"On Tue, Jun 6, 2017 at 3:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Sahil Dua <sahildua2305@gmail.com> writes:\n>\n>> Adds a test for the case when only one parameter is passed to '-m'\n>> (move/rename) option.\n>>\n>> For example - if 'git branch -m bbb' is run, it should rename the\n>> currently checked out branch to bbb. There was no test for this\n>> particular case with only one parameter for -m option. However,\n>> there's one similar test case for -M option.\n>>\n>> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>\n>> ---\n>>  t/t3200-branch.sh | 8 ++++++++\n>>  1 file changed, 8 insertions(+)\n>>\n>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n>> index fe62e7c775da6..7504f14bc52f8 100755\n>> --- a/t/t3200-branch.sh\n>> +++ b/t/t3200-branch.sh\n>> @@ -100,6 +100,14 @@ test_expect_success 'git branch -m n/n n should work' '\n>>       git reflog exists refs/heads/n\n>>  '\n>>\n>> +test_expect_success 'git branch -m bbb should rename checked out branch' '\n>> +     test_when_finished git branch -d bbb &&\n>> +     test_when_finished git checkout master &&\n>> +     git checkout -b aaa &&\n>> +     git branch -m bbb &&\n>> +     git reflog exists refs/heads/bbb\n>> +'\n>\n> Looks almost good.  Don't we want to also make sure that HEAD points\n> at bbb branch, e.g.\n>\n>         git symbolic-ref HEAD >actual &&\n>         echo refs/heads/bbb >expect &&\n>         test_cmp expect actual &&\n>\n> after the \"do we have reflog for the bbb branch?\" test?\n>\n> Also, I suspect that we care about the reflog that is moved to 'bbb'\n> retains entries created for 'aaa'.  So perhaps \"reflog exists\" needs\n> to be replaced by something like\n>\n>         git checkout -b aaa &&\n>         git commit --allow-empty -m \"a new commit\" &&\n>         git rev-parse aaa@{1} >expect &&\n>         git branch -m bbb &&\n>         # the topmost entry is about branch creation\n>         git rev-parse bbb@{2} >actual\n>\n> no?\n\nBoth of these changes make complete sense and in my opinion will\nincrease the quality of the tests if applied to the other tests too. I\nnotice that no other test for this (-m|-M) option checks if the branch\nis actually checked out. Similarly, no other test checks if the\noperation retains reflog entries. They just check if the new reflog\nexists.\n\nDo you think I should make similar changes to the other tests for\n(-m|-M) option as well?\n\n>\n>>  test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n>>       git branch o/o &&\n>>       git branch o/p &&\n>>\n>> --\n>> https://github.com/git/git/pull/371\n"},{"id":"321935","messageId":"0102015c96f631f3-557a7960-2557-4aab-894b-efccf6728f31-000000@eu-west-1.amazonses.com","threadId":"46123","inReplyTo":"0102015c7a68c647-372a5a13-5412-4838-921d-66980bac4099-000000@eu-west-1.amazonses.com","subject":"[PATCH v2] t3200: add test for single parameter passed to -m option","fromName":"Sahil Dua","fromEmail":"sahildua2305@gmail.com","sentAt":"2017-06-11T11:42:16Z","receivedAt":"2017-06-11T11:42:22Z","isPatch":true,"sender":{"key":"sahildua2305@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5206277?v=4"},"body":"Add a test for the case when only one parameter is passed to '-m'\n(move/rename) option.\n\nFor example - if 'git branch -m bbb' is run, it should rename the\ncurrently checked out branch to bbb. There was no test for this\nparticular case with only one parameter for -m option. However,\nthere's one similar test case for -M option.\n\nAdd test for making sure HEAD points to the bbb (new branch name). Also\nadd a test for making sure the reflog that is moved to 'bbb' retains\nentries created for the currently checked out branch.\n\nSigned-off-by: Sahil Dua <sahildua2305@gmail.com>\n---\n t/t3200-branch.sh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex fe62e7c775da6..720400c0e42af 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -100,6 +100,20 @@ test_expect_success 'git branch -m n/n n should work' '\n \tgit reflog exists refs/heads/n\n '\n \n+test_expect_success 'git branch -m bbb should rename checked out branch' '\n+\ttest_when_finished git branch -D bbb &&\n+\ttest_when_finished git checkout master &&\n+\tgit checkout -b aaa &&\n+\tgit commit --allow-empty -m \"a new commit\" &&\n+\tgit rev-parse aaa@{0} >expect &&\n+\tgit branch -m bbb &&\n+\tgit rev-parse bbb@{1} >actual &&\n+\ttest_cmp expect actual &&\n+\tgit symbolic-ref HEAD >actual &&\n+\techo refs/heads/bbb >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n \tgit branch o/o &&\n \tgit branch o/p &&\n\n--\nhttps://github.com/git/git/pull/371\n"},{"id":"321957","messageId":"xmqqy3sxp779.fsf@gitster.mtv.corp.google.com","threadId":"46123","inReplyTo":"0102015c96f631f3-557a7960-2557-4aab-894b-efccf6728f31-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] t3200: add test for single parameter passed to -m option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-12T15:16:26Z","receivedAt":"2017-06-12T15:16:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sahil Dua <sahildua2305@gmail.com> writes:\n\n> Add a test for the case when only one parameter is passed to '-m'\n> (move/rename) option.\n>\n> For example - if 'git branch -m bbb' is run, it should rename the\n> currently checked out branch to bbb. There was no test for this\n> particular case with only one parameter for -m option. However,\n> there's one similar test case for -M option.\n>\n> Add test for making sure HEAD points to the bbb (new branch name). Also\n> add a test for making sure the reflog that is moved to 'bbb' retains\n> entries created for the currently checked out branch.\n>\n> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>\n> ---\n>  t/t3200-branch.sh | 14 ++++++++++++++\n>  1 file changed, 14 insertions(+)\n>\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index fe62e7c775da6..720400c0e42af 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -100,6 +100,20 @@ test_expect_success 'git branch -m n/n n should work' '\n>  \tgit reflog exists refs/heads/n\n>  '\n>  \n> +test_expect_success 'git branch -m bbb should rename checked out branch' '\n> +\ttest_when_finished git branch -D bbb &&\n> +\ttest_when_finished git checkout master &&\n> +\tgit checkout -b aaa &&\n> +\tgit commit --allow-empty -m \"a new commit\" &&\n> +\tgit rev-parse aaa@{0} >expect &&\n> +\tgit branch -m bbb &&\n> +\tgit rev-parse bbb@{1} >actual &&\n> +\ttest_cmp expect actual &&\n\nI think future readers of this test would appreciate if there were\nsome comment explaining why aaa@{0} and bbb@{1} (not bbb@{0}) are\nexpected to be the same here.\n\n> +\tgit symbolic-ref HEAD >actual &&\n> +\techo refs/heads/bbb >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n>  \tgit branch o/o &&\n>  \tgit branch o/p &&\n>\n> --\n> https://github.com/git/git/pull/371\n\nOther than that, looks sensible to me.\n\nThanks.\n"},{"id":"322066","messageId":"0102015ca110a839-f4703e91-1a4d-4d8e-8406-df06b24683da-000000@eu-west-1.amazonses.com","threadId":"46123","inReplyTo":"0102015c96f631f3-557a7960-2557-4aab-894b-efccf6728f31-000000@eu-west-1.amazonses.com","subject":"[PATCH v3] t3200: add test for single parameter passed to -m option","fromName":"Sahil Dua","fromEmail":"sahildua2305@gmail.com","sentAt":"2017-06-13T10:47:22Z","receivedAt":"2017-06-13T10:47:28Z","isPatch":true,"sender":{"key":"sahildua2305@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5206277?v=4"},"body":"Add a test for the case when only one parameter is passed to '-m'\n(move/rename) option.\n\nFor example - if 'git branch -m bbb' is run while checked out on aaa\nbranch, it should rename the currently checked out branch to bbb.\nThere was no test for this particular case with only one parameter\nfor -m option. However, there's one similar test case for -M option.\n\nAdd test for making sure HEAD points to the bbb (new branch name). Also\nadd a test for making sure the reflog that is moved to 'bbb' retains\nentries created for the currently checked out branch. Note that since\nthe topmost entry on reflog for bbb will be about branch creation, we\ncompare bbb@{1} (instead of bbb@{0}) with aaa@{0} to make sure the\nreflog for bbb retains entries from aaa.\n\nSigned-off-by: Sahil Dua <sahildua2305@gmail.com>\n---\n t/t3200-branch.sh | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex fe62e7c775da6..8c9cd70696f87 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -100,6 +100,23 @@ test_expect_success 'git branch -m n/n n should work' '\n \tgit reflog exists refs/heads/n\n '\n \n+# The topmost entry in reflog for branch bbb is about branch creation.\n+# Hence, we compare bbb@{1} (instead of bbb@{0}) with aaa@{0}.\n+\n+test_expect_success 'git branch -m bbb should rename checked out branch' '\n+\ttest_when_finished git branch -D bbb &&\n+\ttest_when_finished git checkout master &&\n+\tgit checkout -b aaa &&\n+\tgit commit --allow-empty -m \"a new commit\" &&\n+\tgit rev-parse aaa@{0} >expect &&\n+\tgit branch -m bbb &&\n+\tgit rev-parse bbb@{1} >actual &&\n+\ttest_cmp expect actual &&\n+\tgit symbolic-ref HEAD >actual &&\n+\techo refs/heads/bbb >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n \tgit branch o/o &&\n \tgit branch o/p &&\n\n--\nhttps://github.com/git/git/pull/371\n"}]}