threads / discuss / 38125

no-xmailer tests fail under Mac OS

Subject: no-xmailer tests fail under Mac OS

## tl;dr

9 messages between Dec 6, 2014 and Dec 11, 2014.

replies: 8people: 4as markdown or json

Michael Blume· Dec 6, 2014, 02:05 UTC · lore
Failures start from
commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)
Author: Luis Henriques <henrix@camandro.org>
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 <henrix@camandro.org>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
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· Dec 6, 2014, 05:34 UTC · re: Michael Blume · lore

Re: no-xmailer tests fail under Mac OS

On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote:
Show 14 quoted lines
> Failures start from
> 
> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)
> Author: Luis Henriques <henrix@camandro.org>
> 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 <henrix@camandro.org>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> 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· Dec 6, 2014, 06:27 UTC · re: Jeff King · lore

Re: no-xmailer tests fail under Mac OS

On Fri, Dec 5, 2014 at 9:34 PM, Jeff King <peff@peff.net> wrote:
Show 35 quoted lines
> On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote:
>
>> Failures start from
>>
>> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)
>> Author: Luis Henriques <henrix@camandro.org>
>> 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 <henrix@camandro.org>
>>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>
>> 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· Dec 6, 2014, 06:32 UTC · re: Michael Blume · lore

Re: no-xmailer tests fail under Mac OS

On Fri, Dec 05, 2014 at 10:27:40PM -0800, Michael Blume wrote:
Show 16 quoted lines
> > 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
Junio C Hamano· Dec 9, 2014, 19:45 UTC · re: Michael Blume · lore

Re: no-xmailer tests fail under Mac OS

Michael Blume <blume.mike@gmail.com> writes:
Show 14 quoted lines
> Failures start from
>
> commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad)
> Author: Luis Henriques <henrix@camandro.org>
> 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 <henrix@camandro.org>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> 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.
Jeff King· Dec 6, 2014, 07:12 UTC · lore

Re: no-xmailer tests fail under Mac OS

On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote:
Show 8 quoted lines
> > 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· Dec 11, 2014, 22:11 UTC · re: Jeff King · lore

Re: no-xmailer tests fail under Mac OS

Jeff King <peff@peff.net> writes:
Show 14 quoted lines
> 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 <nobody@example.com>" \
 		--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· Dec 11, 2014, 22:13 UTC · re: Junio C Hamano · lore

Re: no-xmailer tests fail under Mac OS

On Thu, Dec 11, 2014 at 02:11:04PM -0800, Junio C Hamano wrote:
Show 22 quoted lines
> Jeff King <peff@peff.net> 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· Dec 11, 2014, 22:35 UTC · re: Jeff King · lore

Re: no-xmailer tests fail under Mac OS

Jeff King <peff@peff.net> writes:
Show 28 quoted lines
> On Thu, Dec 11, 2014 at 02:11:04PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> 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

← back to recent threads