Hi Atul,
I took a quick look at your patch and verified that the test case produces an AttributeError on trunk. After manually patching urllib.py the test case passes. A few observations:
1. Your patch changes the code from giving an AttributeError to giving an empty file back when the remote webserver gives a bad http status line. The official Python documentation says that urlopen either generates an IOError when it can't connect to the server, or returns a file-like object when all goes well. In this case all didn't go well, so the documentation is incomplete. Would it be more appropriate to raise an exception rather than return an empty file?
2. I believe your test file will not work on systems without networking support. Other Python tests that require networking support have this at the top of them:
test_support.requires('network')
See Lib/test/test_socketserver.py for example.
3. Consider merging your test into Lib/test/test_urllib.py and supplying a patch against that file.
4. Could the test be performed without actually getting a network connection? Some of the tests in test_urllib.py use a FakeSocket class that might also work for your test.
5. Whitespace police - foo(arg1) instead of foo( arg1 ). See PEP 8 at http://www.python.org/dev/peps/pep-0008/
Other than that, things look good.
|