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

[Senthil Kumaran]

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")
> 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.
> 

> 
> This should also be documented and have a versionchanged added.
> 

Addressed these in r80362 and r80364.

Thanks again.

-- 
Senthil

Q:	Why did the tachyon cross the road?
A:	Because it was on the other side.