# t9000-addresses.sh: unexpected pases 6 messages from 2016-10-21 to 2016-10-21. Participants: Ramsay Jones, Junio C Hamano, Matthieu Moy. Thread: https://gitlist.dev/t/44347 ## Ramsay Jones, 2016-10-21 00:24 Subject: t9000-addresses.sh: unexpected pases Message-ID: <3e9bfa01-d69c-05c1-e24e-960a3add2551@ramsayjones.plus.com> URL: https://gitlist.dev/e/3e9bfa01-d69c-05c1-e24e-960a3add2551%40ramsayjones.plus.com ``` 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, 2016-10-21 00:50 Subject: Re: t9000-addresses.sh: unexpected pases Message-ID: URL: https://gitlist.dev/e/xmqq1szaeda9.fsf%40gitster.mtv.corp.google.com In-Reply-To: <3e9bfa01-d69c-05c1-e24e-960a3add2551@ramsayjones.plus.com> ``` Ramsay Jones writes: > 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, 2016-10-21 09:20 Subject: [PATCH 1/2] t9000-addresses: update expected results after fix Message-ID: <20161021092024.15861-1-Matthieu.Moy@imag.fr> URL: https://gitlist.dev/e/20161021092024.15861-1-Matthieu.Moy%40imag.fr In-Reply-To: ``` 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 --- 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" ], q[Doe, jane ], q["Jane Doe ], - q['Jane 'Doe' ]); + q['Jane 'Doe' ], + q[Jane@:;\.,()<>Doe ], + q[Jane Doe], + q[ Jane Doe]); my @known_failure_list = (q[Jane\ Doe ], q["Doe, Ja"ne ], q["Doe, Katarina" Jane ], - q[Jane@:;\.,()<>Doe ], q[Jane jdoe@example.com], - q[ Jane Doe], - q[Jane Doe], q["Jane "Kat"a" ri"na" ",Doe" ], q[Jane Doe], q[Jane "Doe "], -- 2.10.1.651.gffd0de0 ``` ## Matthieu Moy, 2016-10-21 09:20 Subject: [PATCH 2/2] Git.pm: add comment pointing to t9000 Message-ID: <20161021092024.15861-2-Matthieu.Moy@imag.fr> URL: https://gitlist.dev/e/20161021092024.15861-2-Matthieu.Moy%40imag.fr In-Reply-To: <20161021092024.15861-1-Matthieu.Moy@imag.fr> ``` 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 --- 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, 2016-10-21 16:48 Subject: Re: [PATCH 1/2] t9000-addresses: update expected results after fix Message-ID: URL: https://gitlist.dev/e/xmqqh985d4x7.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20161021092024.15861-1-Matthieu.Moy@imag.fr> ``` Matthieu Moy 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 > --- Thanks. ``` ## Ramsay Jones, 2016-10-21 18:44 Subject: Re: [PATCH 1/2] t9000-addresses: update expected results after fix Message-ID: <3aa70781-e930-c7a9-c106-0268f82f6a5d@ramsayjones.plus.com> URL: https://gitlist.dev/e/3aa70781-e930-c7a9-c106-0268f82f6a5d%40ramsayjones.plus.com In-Reply-To: ``` On 21/10/16 17:48, Junio C Hamano wrote: > Matthieu Moy 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 >> --- > > 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 ```