# no-xmailer tests fail under Mac OS 9 messages from 2014-12-06 to 2014-12-11. Participants: Michael Blume, Jeff King, Junio C Hamano, Luis Henriques. Thread: https://gitlist.dev/t/38125 ## Michael Blume, 2014-12-06 02:05 Subject: no-xmailer tests fail under Mac OS Message-ID: URL: https://gitlist.dev/e/CAO2U3Qg3KsjvWJFza4MfiQDa2LbCpOy1Nxf9Vt3NSgXc7Bee9g%40mail.gmail.com ``` Failures start from commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) Author: Luis Henriques Date: Thu Dec 4 19:11:30 2014 +0000 test/send-email: --[no-]xmailer tests Add tests for the --[no-]xmailer option. Signed-off-by: Luis Henriques Signed-off-by: Junio C Hamano but continue with Junio's SQUASH??? commit at b728d078 Verbose output follows expecting success: do_xmailer_test 1 "--xmailer" && do_xmailer_test 0 "--no-xmailer" 0001-add-master.patch not ok 109 - --[no-]xmailer without any configuration # # do_xmailer_test 1 "--xmailer" && # do_xmailer_test 0 "--no-xmailer" # expecting success: test_config sendemail.xmailer true && do_xmailer_test 1 "" && do_xmailer_test 0 "--no-xmailer" && do_xmailer_test 1 "--xmailer" 0001-add-master.patch not ok 110 - --[no-]xmailer with sendemail.xmailer=true # # test_config sendemail.xmailer true && # do_xmailer_test 1 "" && # do_xmailer_test 0 "--no-xmailer" && # do_xmailer_test 1 "--xmailer" # expecting success: test_config sendemail.xmailer false && do_xmailer_test 0 "" && do_xmailer_test 0 "--no-xmailer" && do_xmailer_test 1 "--xmailer" 0001-add-master.patch not ok 111 - --[no-]xmailer with sendemail.xmailer=false # # test_config sendemail.xmailer false && # do_xmailer_test 0 "" && # do_xmailer_test 0 "--no-xmailer" && # do_xmailer_test 1 "--xmailer" # # failed 3 among 111 test(s) 1..111 ``` ## Jeff King, 2014-12-06 05:34 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: <20141206053434.GF31301@peff.net> URL: https://gitlist.dev/e/20141206053434.GF31301%40peff.net In-Reply-To: ``` On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote: > Failures start from > > commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) > Author: Luis Henriques > Date: Thu Dec 4 19:11:30 2014 +0000 > > test/send-email: --[no-]xmailer tests > > Add tests for the --[no-]xmailer option. > > Signed-off-by: Luis Henriques > Signed-off-by: Junio C Hamano > > but continue with Junio's SQUASH??? commit at b728d078 The commit contains: + test "z$(grep ^X-Mailer: out | wc -l)" = "z$expected" We have had trouble in the past with "wc -l" output not being strictly portable. I do not recall offhand which systems, but it is a good bet that this is the culprit. Doing: grep ^X-Mailer: out >mailer && test_line_count = $expected mailer should fix it. It might be even nicer to actually compare the x-mailer line we find to an expected output, but that may introduce complications if the value changes with the version or something (you'd have to sanitize the output, and then I do not know that the test is really buying much over just seeing whether it exists). -Peff ``` ## Michael Blume, 2014-12-06 06:27 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: URL: https://gitlist.dev/e/CAO2U3QjkJRPrFvapK%3DLDU_GG5Ne6_zVD%2BS61JP%2BiV_Xi-gk5JQ%40mail.gmail.com In-Reply-To: <20141206053434.GF31301@peff.net> ``` On Fri, Dec 5, 2014 at 9:34 PM, Jeff King wrote: > On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote: > >> Failures start from >> >> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) >> Author: Luis Henriques >> Date: Thu Dec 4 19:11:30 2014 +0000 >> >> test/send-email: --[no-]xmailer tests >> >> Add tests for the --[no-]xmailer option. >> >> Signed-off-by: Luis Henriques >> Signed-off-by: Junio C Hamano >> >> but continue with Junio's SQUASH??? commit at b728d078 > > The commit contains: > > + test "z$(grep ^X-Mailer: out | wc -l)" = "z$expected" > > We have had trouble in the past with "wc -l" output not being strictly > portable. I do not recall offhand which systems, but it is a good bet > that this is the culprit. Doing: > > grep ^X-Mailer: out >mailer && > test_line_count = $expected mailer > > should fix it. It might be even nicer to actually compare the x-mailer > line we find to an expected output, but that may introduce complications > if the value changes with the version or something (you'd have to > sanitize the output, and then I do not know that the test is really > buying much over just seeing whether it exists). > > -Peff Actually need to drop the '&&', but yes, that works perfectly, thanks =) ``` ## Jeff King, 2014-12-06 06:32 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: <20141206063245.GA5966@peff.net> URL: https://gitlist.dev/e/20141206063245.GA5966%40peff.net In-Reply-To: ``` On Fri, Dec 05, 2014 at 10:27:40PM -0800, Michael Blume wrote: > > We have had trouble in the past with "wc -l" output not being strictly > > portable. I do not recall offhand which systems, but it is a good bet > > that this is the culprit. Doing: > > > > grep ^X-Mailer: out >mailer && > > test_line_count = $expected mailer > > > > should fix it. It might be even nicer to actually compare the x-mailer > > line we find to an expected output, but that may introduce complications > > if the value changes with the version or something (you'd have to > > sanitize the output, and then I do not know that the test is really > > buying much over just seeing whether it exists). > > > > -Peff > > Actually need to drop the '&&', but yes, that works perfectly, thanks =) Ah, right, we might be looking for 0 sometimes. The right way to do it without destroying the &&-chaining is: { grep ^X-Mailer: out || true } && test_line_count = $expected mailer -Peff ``` ## Jeff King, 2014-12-06 07:12 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: <20141206071234.GA6850@peff.net> URL: https://gitlist.dev/e/20141206071234.GA6850%40peff.net In-Reply-To: ``` On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote: > > Ah, right, we might be looking for 0 sometimes. The right way to do it > > without destroying the &&-chaining is: > > > > { grep ^X-Mailer: out || true } && > > test_line_count = $expected mailer > > Hmm, it doesn't look like that helper is &&-chained though? So it > seems like we could just do without the && You're right, but that is IMHO a bug. We would not notice if send-email or format-patch barfed, and we are expecting to find no X-Mailer (we wouldn't, but for the wrong reason). It should also be using test_config in the last two tests. -Peff ``` ## Junio C Hamano, 2014-12-09 19:45 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: URL: https://gitlist.dev/e/xmqqy4qgli8y.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` Michael Blume writes: > Failures start from > > commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) > Author: Luis Henriques > Date: Thu Dec 4 19:11:30 2014 +0000 > > test/send-email: --[no-]xmailer tests > > Add tests for the --[no-]xmailer option. > > Signed-off-by: Luis Henriques > Signed-off-by: Junio C Hamano > > but continue with Junio's SQUASH??? commit at b728d078 I missed this bit in the patch you pointed out: + test "z$(grep ^X-Mailer: out | wc -l)" = "z$expected" which depends on "wc -l" not to add any extra whitespace around its count. We know that some implementations do, and we should be using test $(grep xxx | wc -l) -eq $expected or something, or test_line_count. Thanks for noticing. ``` ## Junio C Hamano, 2014-12-11 22:11 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: URL: https://gitlist.dev/e/xmqq1to5et1j.fsf%40gitster.dls.corp.google.com In-Reply-To: <20141206071234.GA6850@peff.net> ``` Jeff King writes: > On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote: > >> > Ah, right, we might be looking for 0 sometimes. The right way to do it >> > without destroying the &&-chaining is: >> > >> > { grep ^X-Mailer: out || true } && >> > test_line_count = $expected mailer >> >> Hmm, it doesn't look like that helper is &&-chained though? So it >> seems like we could just do without the && > > You're right, but that is IMHO a bug. We would not notice if send-email > or format-patch barfed, and we are expecting to find no X-Mailer (we > wouldn't, but for the wrong reason). Let me patch this up further by amending the SQUASH??? at the tip. t/t9001-send-email.sh | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index bb573ef..7826aa8 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1408,18 +1408,17 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' ' ' do_xmailer_test() { - expected=$1 - params=$2 - git format-patch -1 + expected=$1 params=$2 && + git format-patch -1 && git send-email \ --from="Example " \ --to=someone@example.com \ --smtp-server="$(pwd)/fake.sendmail" \ $params \ 0001-*.patch \ - 2>errors >out - test "z$(grep ^X-Mailer: out | wc -l)" = "z$expected" - return $? + 2>errors >out && + { grep '^X-Mailer:' out || :; } >mailer && + test_line_count = $expected mailer } test_expect_success $PREREQ '--[no-]xmailer without any configuration' ' ``` ## Jeff King, 2014-12-11 22:13 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: <20141211221339.GA9478@peff.net> URL: https://gitlist.dev/e/20141211221339.GA9478%40peff.net In-Reply-To: ``` On Thu, Dec 11, 2014 at 02:11:04PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote: > > > >> > Ah, right, we might be looking for 0 sometimes. The right way to do it > >> > without destroying the &&-chaining is: > >> > > >> > { grep ^X-Mailer: out || true } && > >> > test_line_count = $expected mailer > >> > >> Hmm, it doesn't look like that helper is &&-chained though? So it > >> seems like we could just do without the && > > > > You're right, but that is IMHO a bug. We would not notice if send-email > > or format-patch barfed, and we are expecting to find no X-Mailer (we > > wouldn't, but for the wrong reason). > > Let me patch this up further by amending the SQUASH??? at the tip. > > t/t9001-send-email.sh | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > [...] Yeah, looks good to me. -Peff ``` ## Luis Henriques, 2014-12-11 22:35 Subject: Re: no-xmailer tests fail under Mac OS Message-ID: <87h9x124s9.fsf@camandro.org> URL: https://gitlist.dev/e/87h9x124s9.fsf%40camandro.org In-Reply-To: <20141211221339.GA9478@peff.net> ``` Jeff King writes: > On Thu, Dec 11, 2014 at 02:11:04PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote: >> > >> >> > Ah, right, we might be looking for 0 sometimes. The right way to do it >> >> > without destroying the &&-chaining is: >> >> > >> >> > { grep ^X-Mailer: out || true } && >> >> > test_line_count = $expected mailer >> >> >> >> Hmm, it doesn't look like that helper is &&-chained though? So it >> >> seems like we could just do without the && >> > >> > You're right, but that is IMHO a bug. We would not notice if send-email >> > or format-patch barfed, and we are expecting to find no X-Mailer (we >> > wouldn't, but for the wrong reason). >> >> Let me patch this up further by amending the SQUASH??? at the tip. >> >> t/t9001-send-email.sh | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 deletions(-) >> [...] > > Yeah, looks good to me. > > -Peff Same here. Thanks a lot for fixing this. Cheers, -- Luís ```