{"thread":{"id":"38125","subject":"no-xmailer tests fail under Mac OS","startedAt":"2014-12-06T02:05:24Z","lastAt":"2014-12-11T22:35:50Z","messageCount":9,"participants":["Michael Blume","Jeff King","Junio C Hamano","Luis Henriques"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"253217","messageId":"CAO2U3Qg3KsjvWJFza4MfiQDa2LbCpOy1Nxf9Vt3NSgXc7Bee9g@mail.gmail.com","threadId":"38125","inReplyTo":null,"subject":"no-xmailer tests fail under Mac OS","fromName":"Michael Blume","fromEmail":"blume.mike@gmail.com","sentAt":"2014-12-06T02:05:24Z","receivedAt":"2014-12-06T02:05:24Z","isPatch":false,"sender":{"key":"blume.mike@gmail.com","avatar":"https://gravatar.com/avatar/1a7b440e1d942425ff4098ac7fc15b86b30cecaa56e1692a7ef8b5939ba25ea7?d=mp&s=160"},"body":"Failures start from\n\ncommit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)\nAuthor: Luis Henriques \nDate: Thu Dec 4 19:11:30 2014 +0000\n\n test/send-email: --[no-]xmailer tests\n\n Add tests for the --[no-]xmailer option.\n\n Signed-off-by: Luis Henriques \n Signed-off-by: Junio C Hamano \n\nbut continue with Junio's SQUASH??? commit at b728d078\n\nVerbose output follows\n\nexpecting success:\ndo_xmailer_test 1 \"--xmailer\" &&\ndo_xmailer_test 0 \"--no-xmailer\"\n\n0001-add-master.patch\nnot ok 109 - --[no-]xmailer without any configuration\n#\n# do_xmailer_test 1 \"--xmailer\" &&\n# do_xmailer_test 0 \"--no-xmailer\"\n#\n\nexpecting success:\ntest_config sendemail.xmailer true &&\ndo_xmailer_test 1 \"\" &&\ndo_xmailer_test 0 \"--no-xmailer\" &&\ndo_xmailer_test 1 \"--xmailer\"\n\n0001-add-master.patch\nnot ok 110 - --[no-]xmailer with sendemail.xmailer=true\n#\n# test_config sendemail.xmailer true &&\n# do_xmailer_test 1 \"\" &&\n# do_xmailer_test 0 \"--no-xmailer\" &&\n# do_xmailer_test 1 \"--xmailer\"\n#\n\nexpecting success:\ntest_config sendemail.xmailer false &&\ndo_xmailer_test 0 \"\" &&\ndo_xmailer_test 0 \"--no-xmailer\" &&\ndo_xmailer_test 1 \"--xmailer\"\n\n0001-add-master.patch\nnot ok 111 - --[no-]xmailer with sendemail.xmailer=false\n#\n# test_config sendemail.xmailer false &&\n# do_xmailer_test 0 \"\" &&\n# do_xmailer_test 0 \"--no-xmailer\" &&\n# do_xmailer_test 1 \"--xmailer\"\n#\n\n# failed 3 among 111 test(s)\n1..111\n"},{"id":"253232","messageId":"20141206053434.GF31301@peff.net","threadId":"38125","inReplyTo":"CAO2U3Qg3KsjvWJFza4MfiQDa2LbCpOy1Nxf9Vt3NSgXc7Bee9g@mail.gmail.com","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-06T05:34:34Z","receivedAt":"2014-12-06T05:34:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote:\n\n> Failures start from\n> \n> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)\n> Author: Luis Henriques \n> Date: Thu Dec 4 19:11:30 2014 +0000\n> \n> test/send-email: --[no-]xmailer tests\n> \n> Add tests for the --[no-]xmailer option.\n> \n> Signed-off-by: Luis Henriques \n> Signed-off-by: Junio C Hamano \n> \n> but continue with Junio's SQUASH??? commit at b728d078\n\nThe commit contains:\n\n + test \"z$(grep ^X-Mailer: out | wc -l)\" = \"z$expected\"\n\nWe have had trouble in the past with \"wc -l\" output not being strictly\nportable. I do not recall offhand which systems, but it is a good bet\nthat this is the culprit. Doing:\n\n grep ^X-Mailer: out >mailer &&\n test_line_count = $expected mailer\n\nshould fix it. It might be even nicer to actually compare the x-mailer\nline we find to an expected output, but that may introduce complications\nif the value changes with the version or something (you'd have to\nsanitize the output, and then I do not know that the test is really\nbuying much over just seeing whether it exists).\n\n-Peff\n"},{"id":"253234","messageId":"CAO2U3QjkJRPrFvapK=LDU_GG5Ne6_zVD+S61JP+iV_Xi-gk5JQ@mail.gmail.com","threadId":"38125","inReplyTo":"20141206053434.GF31301@peff.net","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Michael Blume","fromEmail":"blume.mike@gmail.com","sentAt":"2014-12-06T06:27:40Z","receivedAt":"2014-12-06T06:27:40Z","isPatch":false,"sender":{"key":"blume.mike@gmail.com","avatar":"https://gravatar.com/avatar/1a7b440e1d942425ff4098ac7fc15b86b30cecaa56e1692a7ef8b5939ba25ea7?d=mp&s=160"},"body":"On Fri, Dec 5, 2014 at 9:34 PM, Jeff King wrote:\n> On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote:\n>\n>> Failures start from\n>>\n>> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)\n>> Author: Luis Henriques \n>> Date: Thu Dec 4 19:11:30 2014 +0000\n>>\n>> test/send-email: --[no-]xmailer tests\n>>\n>> Add tests for the --[no-]xmailer option.\n>>\n>> Signed-off-by: Luis Henriques \n>> Signed-off-by: Junio C Hamano \n>>\n>> but continue with Junio's SQUASH??? commit at b728d078\n>\n> The commit contains:\n>\n> + test \"z$(grep ^X-Mailer: out | wc -l)\" = \"z$expected\"\n>\n> We have had trouble in the past with \"wc -l\" output not being strictly\n> portable. I do not recall offhand which systems, but it is a good bet\n> that this is the culprit. Doing:\n>\n> grep ^X-Mailer: out >mailer &&\n> test_line_count = $expected mailer\n>\n> should fix it. It might be even nicer to actually compare the x-mailer\n> line we find to an expected output, but that may introduce complications\n> if the value changes with the version or something (you'd have to\n> sanitize the output, and then I do not know that the test is really\n> buying much over just seeing whether it exists).\n>\n> -Peff\n\nActually need to drop the '&&', but yes, that works perfectly, thanks =)\n"},{"id":"253235","messageId":"20141206063245.GA5966@peff.net","threadId":"38125","inReplyTo":"CAO2U3QjkJRPrFvapK=LDU_GG5Ne6_zVD+S61JP+iV_Xi-gk5JQ@mail.gmail.com","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-06T06:32:46Z","receivedAt":"2014-12-06T06:32:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 05, 2014 at 10:27:40PM -0800, Michael Blume wrote:\n\n> > We have had trouble in the past with \"wc -l\" output not being strictly\n> > portable. I do not recall offhand which systems, but it is a good bet\n> > that this is the culprit. Doing:\n> >\n> > grep ^X-Mailer: out >mailer &&\n> > test_line_count = $expected mailer\n> >\n> > should fix it. It might be even nicer to actually compare the x-mailer\n> > line we find to an expected output, but that may introduce complications\n> > if the value changes with the version or something (you'd have to\n> > sanitize the output, and then I do not know that the test is really\n> > buying much over just seeing whether it exists).\n> >\n> > -Peff\n> \n> Actually need to drop the '&&', but yes, that works perfectly, thanks =)\n\nAh, right, we might be looking for 0 sometimes. The right way to do it\nwithout destroying the &&-chaining is:\n\n { grep ^X-Mailer: out || true } &&\n test_line_count = $expected mailer\n\n-Peff\n"},{"id":"253236","messageId":"20141206071234.GA6850@peff.net","threadId":"38125","inReplyTo":"CAO2U3QgDMpKwqsjzPNECpJw4z+WbboX5ug7Shu5v5ZCuPsKuGQ@mail.gmail.com","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-06T07:12:34Z","receivedAt":"2014-12-06T07:12:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote:\n\n> > Ah, right, we might be looking for 0 sometimes. The right way to do it\n> > without destroying the &&-chaining is:\n> >\n> > { grep ^X-Mailer: out || true } &&\n> > test_line_count = $expected mailer\n> \n> Hmm, it doesn't look like that helper is &&-chained though? So it\n> seems like we could just do without the &&\n\nYou're right, but that is IMHO a bug. We would not notice if send-email\nor format-patch barfed, and we are expecting to find no X-Mailer (we\nwouldn't, but for the wrong reason).\n\nIt should also be using test_config in the last two tests.\n\n-Peff\n"},{"id":"253434","messageId":"xmqqy4qgli8y.fsf@gitster.dls.corp.google.com","threadId":"38125","inReplyTo":"CAO2U3Qg3KsjvWJFza4MfiQDa2LbCpOy1Nxf9Vt3NSgXc7Bee9g@mail.gmail.com","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-09T19:45:33Z","receivedAt":"2014-12-09T19:45:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Blume writes:\n\n> Failures start from\n>\n> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)\n> Author: Luis Henriques \n> Date: Thu Dec 4 19:11:30 2014 +0000\n>\n> test/send-email: --[no-]xmailer tests\n>\n> Add tests for the --[no-]xmailer option.\n>\n> Signed-off-by: Luis Henriques \n> Signed-off-by: Junio C Hamano \n>\n> but continue with Junio's SQUASH??? commit at b728d078\n\nI missed this bit in the patch you pointed out:\n\n+ test \"z$(grep ^X-Mailer: out | wc -l)\" = \"z$expected\"\n\nwhich depends on \"wc -l\" not to add any extra whitespace around its\ncount. We know that some implementations do, and we should be using\n\n\ttest $(grep xxx | wc -l) -eq $expected\n\nor something, or test_line_count.\n\nThanks for noticing.\n"},{"id":"253583","messageId":"xmqq1to5et1j.fsf@gitster.dls.corp.google.com","threadId":"38125","inReplyTo":"20141206071234.GA6850@peff.net","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-11T22:11:04Z","receivedAt":"2014-12-11T22:11:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote:\n>\n>> > Ah, right, we might be looking for 0 sometimes. The right way to do it\n>> > without destroying the &&-chaining is:\n>> >\n>> > { grep ^X-Mailer: out || true } &&\n>> > test_line_count = $expected mailer\n>> \n>> Hmm, it doesn't look like that helper is &&-chained though? So it\n>> seems like we could just do without the &&\n>\n> You're right, but that is IMHO a bug. We would not notice if send-email\n> or format-patch barfed, and we are expecting to find no X-Mailer (we\n> wouldn't, but for the wrong reason).\n\nLet me patch this up further by amending the SQUASH??? at the tip.\n\n t/t9001-send-email.sh | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex bb573ef..7826aa8 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1408,18 +1408,17 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '\n '\n \n do_xmailer_test() {\n-\texpected=$1\n-\tparams=$2\n-\tgit format-patch -1\n+\texpected=$1 params=$2 &&\n+\tgit format-patch -1 &&\n \tgit send-email \\\n \t\t--from=\"Example \" \\\n \t\t--to=someone@example.com \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t$params \\\n \t\t0001-*.patch \\\n-\t\t2>errors >out\n-\ttest \"z$(grep ^X-Mailer: out | wc -l)\" = \"z$expected\"\n-\treturn $?\n+\t\t2>errors >out &&\n+\t{ grep '^X-Mailer:' out || :; } >mailer &&\n+\ttest_line_count = $expected mailer\n }\n \n test_expect_success $PREREQ '--[no-]xmailer without any configuration' '\n"},{"id":"253584","messageId":"20141211221339.GA9478@peff.net","threadId":"38125","inReplyTo":"xmqq1to5et1j.fsf@gitster.dls.corp.google.com","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-11T22:13:39Z","receivedAt":"2014-12-11T22:13:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 11, 2014 at 02:11:04PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote:\n> >\n> >> > Ah, right, we might be looking for 0 sometimes. The right way to do it\n> >> > without destroying the &&-chaining is:\n> >> >\n> >> > { grep ^X-Mailer: out || true } &&\n> >> > test_line_count = $expected mailer\n> >> \n> >> Hmm, it doesn't look like that helper is &&-chained though? So it\n> >> seems like we could just do without the &&\n> >\n> > You're right, but that is IMHO a bug. We would not notice if send-email\n> > or format-patch barfed, and we are expecting to find no X-Mailer (we\n> > wouldn't, but for the wrong reason).\n> \n> Let me patch this up further by amending the SQUASH??? at the tip.\n> \n> t/t9001-send-email.sh | 11 +++++------\n> 1 file changed, 5 insertions(+), 6 deletions(-)\n> [...]\n\nYeah, looks good to me.\n\n-Peff\n"},{"id":"253585","messageId":"87h9x124s9.fsf@camandro.org","threadId":"38125","inReplyTo":"20141211221339.GA9478@peff.net","subject":"Re: no-xmailer tests fail under Mac OS","fromName":"Luis Henriques","fromEmail":"henrix@camandro.org","sentAt":"2014-12-11T22:35:50Z","receivedAt":"2014-12-11T22:35:50Z","isPatch":false,"sender":{"key":"henrix@camandro.org","avatar":"https://avatars.githubusercontent.com/u/73643340?v=4"},"body":"Jeff King writes:\n\n> On Thu, Dec 11, 2014 at 02:11:04PM -0800, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote:\n>> >\n>> >> > Ah, right, we might be looking for 0 sometimes. The right way to do it\n>> >> > without destroying the &&-chaining is:\n>> >> >\n>> >> > { grep ^X-Mailer: out || true } &&\n>> >> > test_line_count = $expected mailer\n>> >> \n>> >> Hmm, it doesn't look like that helper is &&-chained though? So it\n>> >> seems like we could just do without the &&\n>> >\n>> > You're right, but that is IMHO a bug. We would not notice if send-email\n>> > or format-patch barfed, and we are expecting to find no X-Mailer (we\n>> > wouldn't, but for the wrong reason).\n>> \n>> Let me patch this up further by amending the SQUASH??? at the tip.\n>> \n>> t/t9001-send-email.sh | 11 +++++------\n>> 1 file changed, 5 insertions(+), 6 deletions(-)\n>> [...]\n>\n> Yeah, looks good to me.\n>\n> -Peff\n\nSame here. Thanks a lot for fixing this.\n\nCheers,\n--\nLuís\n"}]}