[Python-checkins] r80101 - in python/trunk: Lib/test/test_urlparse.py Lib/urlparse.py Misc/NEWS

[Walter Dörwald]

On 22.04.10 14:21, Senthil Kumaran wrote:

> On Mon, Apr 19, 2010 at 11:31:00AM +0300, Ezio Melotti wrote:
>>
>> For all these 3 invalid URLs the exception is raised by
>> urlparse.urlparse(), so the one raised by .hostname is not tested
>> (and the two assertRaises are equivalent).
>> If it's possible to parse an URL and have the exception raised by
>> .hostname, a separate test should check that the urlparse()
>> succeeded and that getting the hostname raises an exception. If it's
>> not possible, maybe the raise in .hostname [1] should be an assert.
>> Also you can use self.assertRaises(ValueError, urlparse.urlparse,
>> invalid_url) instead of the lambda, or even:
>> result = urlparse.urlparse(invalid_url)
>> with self.assertRaises(ValueError):
>>     result.hostname
>> on 2.7/3.2.
> 
>> Isn't this equivalent to:
>> if (('[' in url and ']' not in url) or
>>     (']' in url and '[' not in url)):
>>     raise ValueError("Invalid IPv6 URL")

And this is equivalent to:

if ('[' in url) != (']' in url):
    raise ValueError("Invalid IPv6 URL")

>> I find this version more readable and it avoids 4 if and the
>> duplication of the raise. The 'delim = len(url)' can also be moved
>> after this check.
>>
> [...]
> Addressed these in r80362 and r80364.
> 
> Thanks again.

Servus,
   Walter