{"thread":{"id":"54158","subject":"[PATCH] t3200: clean side effect of git checkout --orphan","startedAt":"2020-08-29T22:58:10Z","lastAt":"2020-08-31T18:22:00Z","messageCount":4,"participants":["Aaron Lipman","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"404762","messageId":"20200829225648.11971-1-alipman88@gmail.com","threadId":"54158","inReplyTo":null,"subject":"[PATCH] t3200: clean side effect of git checkout --orphan","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-29T22:56:48Z","receivedAt":"2020-08-29T22:58:10Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"The \"refuse --edit-description on unborn branch for now\" test in t3200\nswitches to an orphan branch, causing subsequent git commands\nreferencing HEAD to fail. Avoid this side-effect by switching back to\nmaster after that test finishes.\n\nThis has gone undetected, as the next effected test expects failure -\nbut it currently fails for the wrong reason.\n\nVerbose output of the next test referencing HEAD,\n\"--merged is incompatible with --no-merged\":\n\n fatal: malformed object name HEAD\n\nWhich this commit corrects to:\n\n error: option `no-merged' is incompatible with --merged\n\nSigned-off-by: Aaron Lipman \n---\nWe might considered updating \"--merged is incompatible with --no-merged\"\ntests to not only test for failure, but check the contents of the error\nmessage to ensure they fail for the correct reason, e.g.:\n\ntest_expect_success '--merged is incompatible with --no-merged' '\n test_must_fail git branch --merged HEAD --no-merged HEAD 2>error &&\n test_i18ngrep \"is incompatible with --merged\" error\n'\n\nHowever, I intend to submit a patch enabling ref-filter to accept\nmultiple merged/no-merged filters, which would make said updates\nirrelevant.\n\n t/t3200-branch.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 4c0734157b..028c88d1b2 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1287,6 +1287,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n '\n \n test_expect_success 'refuse --edit-description on unborn branch for now' '\n+\ttest_when_finished \"git checkout master\" &&\n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"404764","messageId":"CAPig+cQeYz9Mh+26YshuCQSzXCCUyKNGGr1wJ3FNNLpf=9QRuw@mail.gmail.com","threadId":"54158","inReplyTo":"20200829225648.11971-1-alipman88@gmail.com","subject":"Re: [PATCH] t3200: clean side effect of git checkout --orphan","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-30T05:35:05Z","receivedAt":"2020-08-30T05:35:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Aug 29, 2020 at 6:57 PM Aaron Lipman wrote:\n> The \"refuse --edit-description on unborn branch for now\" test in t3200\n> switches to an orphan branch, causing subsequent git commands\n> referencing HEAD to fail. Avoid this side-effect by switching back to\n> master after that test finishes.\n>\n> This has gone undetected, as the next effected test expects failure -\n> but it currently fails for the wrong reason.\n\ns/effected/affected\n\nIn fact, the three tests following the orphan test all expect failure\n(though I didn't check if they also fail for the wrong reason), and\nthe following test which doesn't expect failure has an explicit \"git\ncheckout master\" early on, which explains why it was not impacted by\nthis problem.\n\n> Verbose output of the next test referencing HEAD,\n> \"--merged is incompatible with --no-merged\":\n>\n> fatal: malformed object name HEAD\n>\n> Which this commit corrects to:\n>\n> error: option `no-merged' is incompatible with --merged\n>\n> Signed-off-by: Aaron Lipman \n\nDescription and actual fix make perfect sense.\n"},{"id":"404778","messageId":"20200830224200.21103-1-alipman88@gmail.com","threadId":"54158","inReplyTo":"20200829225648.11971-1-alipman88@gmail.com","subject":"[PATCH v2] t3200: clean side effect of git checkout --orphan","fromName":"Aaron Lipman","fromEmail":"alipman88@gmail.com","sentAt":"2020-08-30T22:42:00Z","receivedAt":"2020-08-30T22:43:14Z","isPatch":true,"sender":{"key":"alipman88@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3922511?v=4"},"body":"The \"refuse --edit-description on unborn branch for now\" test in t3200\nswitches to an orphan branch, causing subsequent git commands\nreferencing HEAD to fail. Avoid this side-effect by switching back to\nmaster after the test finishes.\n\nThis has gone undetected, as the next affected test expects failure -\nbut it currently fails for the wrong reason.\n\nVerbose output of the next test referencing HEAD,\n\"--merged is incompatible with --no-merged\":\n\n fatal: malformed object name HEAD\n\nWhich this commit corrects to:\n\n error: option `no-merged' is incompatible with --merged\n\nSigned-off-by: Aaron Lipman \n---\nWhoops, fixed the effected/affected error.\nThanks for catching that, Eric!\n\n t/t3200-branch.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 4c0734157b..028c88d1b2 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1287,6 +1287,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n '\n \n test_expect_success 'refuse --edit-description on unborn branch for now' '\n+\ttest_when_finished \"git checkout master\" &&\n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n-- \n2.24.3 (Apple Git-128)\n\n"},{"id":"404824","messageId":"xmqqimcyfzgg.fsf@gitster.c.googlers.com","threadId":"54158","inReplyTo":"CAPig+cQeYz9Mh+26YshuCQSzXCCUyKNGGr1wJ3FNNLpf=9QRuw@mail.gmail.com","subject":"Re: [PATCH] t3200: clean side effect of git checkout --orphan","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-31T18:21:51Z","receivedAt":"2020-08-31T18:22:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Sat, Aug 29, 2020 at 6:57 PM Aaron Lipman wrote:\n>> The \"refuse --edit-description on unborn branch for now\" test in t3200\n>> switches to an orphan branch, causing subsequent git commands\n>> referencing HEAD to fail. Avoid this side-effect by switching back to\n>> master after that test finishes.\n>>\n>> This has gone undetected, as the next effected test expects failure -\n>> but it currently fails for the wrong reason.\n>\n> s/effected/affected\n>\n> In fact, the three tests following the orphan test all expect failure\n> (though I didn't check if they also fail for the wrong reason), and\n> the following test which doesn't expect failure has an explicit \"git\n> checkout master\" early on, which explains why it was not impacted by\n> this problem.\n>\n>> Verbose output of the next test referencing HEAD,\n>> \"--merged is incompatible with --no-merged\":\n>>\n>> fatal: malformed object name HEAD\n>>\n>> Which this commit corrects to:\n>>\n>> error: option `no-merged' is incompatible with --merged\n>>\n>> Signed-off-by: Aaron Lipman \n>\n> Description and actual fix make perfect sense.\n\nYeah, looks good. Of course, the affected test can be made more\ndefensive to protect the precondition it relies on from getting\nbroken by other tests (i.e. if it refers to HEAD, it should make\nsure it is on an actual commit). Each test cleaning up after\nitself is a good discipline to have, but what is \"clean\" is quite\nsubjective and depends on each test piece in the script X-<.\n\nThanks, will queue.\n"}]}