librelist archives

Re: [hg.review] FAIL: review.tests.test_signoff.test_multiple_signoffs

From:

Steve Losh

Date:

2010-07-22 @ 22:28

I pushed a fix for this yesterday -- can you see if it still fails?

On Jul 21, 2010, at 3:02 AM, André Sintzoff wrote:

> Hello,
> 
> With the current tip (375:53aadb5be2b6) of hg-review, I face one
> failure with nosetests if username contains non ASCII characters.
> 
> as@g050202:~$ head -2 .hgrc
> [ui]
> username = André
> 
> 
> as@g050202:~/Development/hg-review/review/tests$ nosetests
> 
................................................./home/as/Development/hg-review/review/api.py:469:
> UnicodeWarning: Unicode equal comparison failed to convert both
> arguments to Unicode - interpreting them as being unequal
> return filter(lambda s: s.author == username, self.signoffs)
> F...
> ======================================================================
> FAIL: review.tests.test_signoff.test_multiple_signoffs
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/usr/lib/pymodules/python2.6/nose/case.py", line 183, in runTest
>   self.test(*self.arg)
> File "/home/as/Development/hg-review/review/tests/test_signoff.py",
> line 72, in test_multiple_signoffs
>   should_fail_with(messages.SIGNOFF_EXISTS, signoff=True,
> message='Test signoff two.')
> File "/home/as/Development/hg-review/review/tests/util.py", line
> 131, in should_fail_with
>   assert False, BAD_ERROR
> AssertionError: The correct error message was not printed.
> -------------------- >> begin captured stdout << ---------------------
> ajout de always_changing
> ajout de always_changing2
> ajout de file_one
> ajout de file_two
> ajout de long_file
> ajout de file_three
> ajout de test_dir/test_file
> the review data has been initialized
> run "hg commit .hgreview -m 'initialize code review'" to record it permanently
> 
> 
> 
> 
> --------------------- >> end captured stdout << ----------------------
> 
> ----------------------------------------------------------------------
> Ran 53 tests in 20.529s
> 
> FAILED (failures=1)
> 
> 
> André, the Unicode tester ;-)

2010/7/23 Steve Losh <steve@stevelosh.com>:
> I pushed a fix for this yesterday -- can you see if it still fails?

nosetests are running correctly.
Thanks.

Regarding the test_username_encoding_(utf8|iso_8859_1) you add, I
would change the email address.
According to RFC 5322
(http://tools.ietf.org/html/rfc5322#section-3.4.1), a correct email
address may not contain non ASCII characters.
Note: IETF is currently discussing how to internationalize email
address (http://tools.ietf.org/wg/eai/)

I suggest you replace tést@test.com by test@test.com

André

On Jul 23, 2010, at 2:55 AM, André Sintzoff wrote:

> 2010/7/23 Steve Losh <steve@stevelosh.com>:
>> I pushed a fix for this yesterday -- can you see if it still fails?
> 
> nosetests are running correctly.
> Thanks.
> 
> Regarding the test_username_encoding_(utf8|iso_8859_1) you add, I
> would change the email address.
> According to RFC 5322
> (http://tools.ietf.org/html/rfc5322#section-3.4.1), a correct email
> address may not contain non ASCII characters.
> Note: IETF is currently discussing how to internationalize email
> address (http://tools.ietf.org/wg/eai/)
> 
> I suggest you replace tést@test.com by test@test.com

Mercurial will let you use the non-ascii email, so I think hg-review 
should allow it (and test for it) too.

> 
> André

2010/7/24 Steve Losh <steve@stevelosh.com>:
> On Jul 23, 2010, at 2:55 AM, André Sintzoff wrote:
>
>> 2010/7/23 Steve Losh <steve@stevelosh.com>:
>>> I pushed a fix for this yesterday -- can you see if it still fails?
>>
>> nosetests are running correctly.
>> Thanks.
>>
>> Regarding the test_username_encoding_(utf8|iso_8859_1) you add, I
>> would change the email address.
>> According to RFC 5322
>> (http://tools.ietf.org/html/rfc5322#section-3.4.1), a correct email
>> address may not contain non ASCII characters.
>> Note: IETF is currently discussing how to internationalize email
>> address (http://tools.ietf.org/wg/eai/)
>>
>> I suggest you replace tést@test.com by test@test.com
>
> Mercurial will let you use the non-ascii email, so I think hg-review 
should allow it (and test for it) too.

OK.

André