threads / discuss / 44347

t9000-addresses.sh: unexpected pases

Subject: t9000-addresses.sh: unexpected pases

## tl;dr

6 messages between Oct 21, 2016 and Oct 21, 2016.

replies: 5people: 3as markdown or json

Ramsay Jones· Oct 21, 2016, 00:24 UTC · lore

Hi Matthieu,

I have started seeing unexpected passes in this test (am I the only one?) on the next and pu branch, which seems to be caused by commit e3fdbcc8 ("parse_mailboxes: accept extra text after <...> address", 13-10-2016). Thus:

  $ tail -15 ntest-out
  [15:17:44]
  All tests successful.

  Test Summary Report
  -------------------
  t9000-addresses.sh                               (Wstat: 0 Tests: 37 Failed: 0)
    TODO passed:   28, 30-31
  Files=760, Tests=13940, 484 wallclock secs ( 4.04 usr  1.30 sys + 60.52 cusr 36.76 csys = 102.62 CPU)
  Result: PASS
  make clean-except-prove-cache
  make[2]: Entering directory '/home/ramsay/git/t'
  rm -f -r 'trash directory'.* 'test-results'
  rm -f -r valgrind/bin
  make[2]: Leaving directory '/home/ramsay/git/t'
  make[1]: Leaving directory '/home/ramsay/git/t'
  $

I have not even looked, but I suspect that it simply requires a change from expect_fail to expect_success, since your commit has 'fixed' these tests ... would you mind taking a quick look?

Thanks!

ATB, Ramsay Jones

Junio C Hamano· Oct 21, 2016, 00:50 UTC · re: Ramsay Jones · lore

Re: t9000-addresses.sh: unexpected pases

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 13 quoted lines

> I have started seeing unexpected passes in this test (am I the
> only one?) on the next and pu branch, which seems to be caused
> by commit e3fdbcc8 ("parse_mailboxes: accept extra text after
> <...> address", 13-10-2016). Thus:
>
>   $ tail -15 ntest-out
>   [15:17:44]
>   All tests successful.
>
>   Test Summary Report
>   -------------------
>   t9000-addresses.sh                               (Wstat: 0 Tests: 37 Failed: 0)
>     TODO passed:   28, 30-31

Yeah, I noticed this in some of my integration runs but didn't pay attention and forgot about it; thanks for bringing it up.

Matthieu Moy· Oct 21, 2016, 09:20 UTC · re: Junio C Hamano · lore

[PATCH 1/2] t9000-addresses: update expected results after fix

e3fdbcc8e1 (parse_mailboxes: accept extra text after <...> address, 2016-10-13) improved our in-house address parser and made it closer to Mail::Address. As a consequence, some tests comparing it to Mail::Address now pass, but e3fdbcc8e1 forgot to update the test.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t9000/test.pl | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t9000/test.pl b/t/t9000/test.pl
index 2d05d3eeab..dfeaa9c655 100755
--- a/t/t9000/test.pl
+++ b/t/t9000/test.pl
@@ -32,15 +32,15 @@ my @success_list = (q[Jane],
 	q["Jane\" Doe" <jdoe@example.com>],
 	q[Doe, jane <jdoe@example.com>],
 	q["Jane Doe <jdoe@example.com>],
-	q['Jane 'Doe' <jdoe@example.com>]);
+	q['Jane 'Doe' <jdoe@example.com>],
+	q[Jane@:;\.,()<>Doe <jdoe@example.com>],
+	q[Jane <jdoe@example.com> Doe],
+	q[<jdoe@example.com> Jane Doe]);
 
 my @known_failure_list = (q[Jane\ Doe <jdoe@example.com>],
 	q["Doe, Ja"ne <jdoe@example.com>],
 	q["Doe, Katarina" Jane <jdoe@example.com>],
-	q[Jane@:;\.,()<>Doe <jdoe@example.com>],
 	q[Jane jdoe@example.com],
-	q[<jdoe@example.com> Jane Doe],
-	q[Jane <jdoe@example.com> Doe],
 	q["Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com>],
 	q[Jane Doe],
 	q[Jane "Doe <jdoe@example.com>"],

-- 
2.10.1.651.gffd0de0

Matthieu Moy· Oct 21, 2016, 09:20 UTC · re: Matthieu Moy · lore

[PATCH 2/2] Git.pm: add comment pointing to t9000

parse_mailboxes should probably eventually be completely equivalent to Mail::Address, and if this happens we can drop the Mail::Address dependency. Add a comment in the code reminding the current state of the code, and point to the corresponding failing test to help future contributors to get it right.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 perl/Git.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/perl/Git.pm b/perl/Git.pm
index 42e0895ef7..8bb2b7c7e3 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -870,6 +870,8 @@ Return an array of mailboxes extracted from a string.
 
 =cut
 
+# Very close to Mail::Address's parser, but we still have minor
+# differences in some cases (see t9000 for examples).
 sub parse_mailboxes {
 	my $re_comment = qr/\((?:[^)]*)\)/;
 	my $re_quote = qr/"(?:[^\"\\]|\\.)*"/;

-- 
2.10.1.651.gffd0de0

Junio C Hamano· Oct 21, 2016, 16:48 UTC · re: Matthieu Moy · lore

Re: [PATCH 1/2] t9000-addresses: update expected results after fix

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 7 quoted lines

> e3fdbcc8e1 (parse_mailboxes: accept extra text after <...> address,
> 2016-10-13) improved our in-house address parser and made it closer to
> Mail::Address. As a consequence, some tests comparing it to
> Mail::Address now pass, but e3fdbcc8e1 forgot to update the test.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

Thanks.

Ramsay Jones· Oct 21, 2016, 18:44 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] t9000-addresses: update expected results after fix

On 21/10/16 17:48, Junio C Hamano wrote:

Show 11 quoted lines

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> 
>> e3fdbcc8e1 (parse_mailboxes: accept extra text after <...> address,
>> 2016-10-13) improved our in-house address parser and made it closer to
>> Mail::Address. As a consequence, some tests comparing it to
>> Mail::Address now pass, but e3fdbcc8e1 forgot to update the test.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
> 
> Thanks.

Yep, thanks for looking into this Matthieu.

I applied these cleanly (to both next and pu) and tested on Linux and cygwin.

Thanks again.

ATB, Ramsay Jones