classification
Title: socket.getaddrinfo needs tests
Type: behavior Stage:
Components: Library (Lib), Tests Versions: Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: exarkun, flox, giampaolo.rodola, loewis, mark.dickinson, pitrou, ronaldoussoren, skrah, vstinner
Priority: high Keywords: buildbot, patch

Created on 2010-05-30 16:58 by pitrou, last changed 2010-08-16 20:56 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
doc-getaddrinfo.patch pitrou, 2010-05-30 18:04
getaddrinfo.patch giampaolo.rodola, 2010-06-17 14:35
getaddrinfo3.patch pitrou, 2010-08-12 14:50
getaddrinfo3.patch giampaolo.rodola, 2010-08-14 15:13 adds skip test as libc bug workaround
getaddrtests.patch giampaolo.rodola, 2010-08-16 15:05
Messages (27)
msg106767 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-30 16:58
socket.getaddrinfo has no tests at all. This should be fixed.
msg106770 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-30 17:23
It also needs better documentation, by the way.
msg106771 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-30 18:01
Here is a possible doc patch for getaddrinfo(). Comments?
msg106794 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-05-31 16:45
About the doc patch: I like the word "Resolves" more than "Translate".  "Resolves" implies possible network activity to me.  "Translate" sounds like it's just a change in representation.  Of course, things like `AI_NUMERICHOST` complicate things, since there may not actually be any network activity.

The rest seems fine.
msg106795 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-31 17:07
Thanks for the comments. Other functions use "translate" too (gethostbyname, getservbyname, etc.), so I preferred to keep it for consistency. I've now committed the doc patch.
msg106860 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-01 20:42
A patch which adds some basic tests is in attachment.
It basically tests what told in the documentation without going too deeper.
msg107677 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-06-12 17:49
Testing the patch under a Windows XP Qemu virtual machine gives the following error. Works fine under 64-bit Linux.

======================================================================
ERROR: testGetaddrinfo (test.test_socket.GeneralModuleTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\cpython\__svn__\lib\test\test_socket.py", line 568, in testGetaddrinf
o
    socket.getaddrinfo('::1', 80)
gaierror: [Errno 11001] getaddrinfo failed

----------------------------------------------------------------------
Ran 99 tests in 14.851s

FAILED (errors=1)
test test_socket failed -- Traceback (most recent call last):
  File "Z:\cpython\__svn__\lib\test\test_socket.py", line 568, in testGetaddrinf
o
    socket.getaddrinfo('::1', 80)
gaierror: [Errno 11001] getaddrinfo failed
msg107684 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-12 19:18
Does that mean has_ipv6() is broken maybe?
I've just tested it against a Windows XP sp3 without IPv6 installed (socket.has_ipv6 returns True thought) and it doesn't fail.
msg107810 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2010-06-14 20:25
FreeBSD/Qemu: ipv6 is ok, but this fails:

======================================================================
FAIL: testGetaddrinfo (__main__.GeneralModuleTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Lib/test/test_socket.py", line 587, in testGetaddrinfo
    self.assertEqual(socktype, socket.SOCK_STREAM)
AssertionError: 2 != 1

----------------------------------------------------------------------
Ran 113 tests in 4.174s

FAILED (failures=1, skipped=79)
Traceback (most recent call last):
  File "Lib/test/test_socket.py", line 1530, in <module>
    test_main()
  File "Lib/test/test_socket.py", line 1526, in test_main
    support.run_unittest(*tests)
  File "/usr/home/stefan/svn/py3k/Lib/test/support.py", line 1054, in run_unittest
    _run_suite(suite)
  File "/usr/home/stefan/svn/py3k/Lib/test/support.py", line 1037, in _run_suite
    raise TestFailed(err)
test.support.TestFailed: Traceback (most recent call last):
  File "Lib/test/test_socket.py", line 587, in testGetaddrinfo
    self.assertEqual(socktype, socket.SOCK_STREAM)
AssertionError: 2 != 1
msg107968 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-17 00:16
> FreeBSD/Qemu: ipv6 is ok, but this fails:
[...]

That failure refers to this test:

        # by specifying "http" we expect all returned sockets have
        # STREAM type
        infos = socket.getaddrinfo(HOST, "http")
        for _, socktype, _, _, _ in infos:
             self.assertEqual(socktype, socket.SOCK_STREAM)

...and means that getaddrinfo() consider UDP (socket.SOCK_DGRAM == 1) a valid transfer protocol for HTTP.
By googling around it seems this might actually be true:
http://stackoverflow.com/questions/323351/does-http-use-udp

Changing the string to "ftp" should solve the problem.
As for the IPv6 failure I added some extra code which executes the test only after verifying that binding an IPv6 socket is actually possible.

Btw, socket.has_ipv6 documentation should be more clear about the fact that having it == True doesn't necessarily mean IPv6 is actually supported.
msg107969 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-17 00:32
Forgot to include socket.gaierror in the list of exceptions.
New patch in attachment.
msg107994 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2010-06-17 08:10
> Btw, socket.has_ipv6 documentation should be more clear about the fact
> that having it == True doesn't necessarily mean IPv6 is actually
> supported.

Strange indeed. socket.has_ipv6 checks whether ENABLE_IPV6 was defined at
compile time. But why is that an attribute of a socket object? It can be
checked in sysconfig.



Unfortunately, the socktype test still fails on FreeBSD/Qemu:

======================================================================
FAIL: testGetaddrinfo (__main__.GeneralModuleTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Lib/test/test_socket.py", line 591, in testGetaddrinfo
    self.assertEqual(socktype, socket.SOCK_STREAM)
AssertionError: 2 != 1

----------------------------------------------------------------------
Ran 101 tests in 11.504s

FAILED (failures=1)
Traceback (most recent call last):
  File "Lib/test/test_socket.py", line 1463, in <module>
    test_main()
  File "Lib/test/test_socket.py", line 1459, in test_main
    test_support.run_unittest(*tests)
  File "/usr/home/stefan/svn/trunk/Lib/test/test_support.py", line 1055, in run_unittest
    _run_suite(suite)
  File "/usr/home/stefan/svn/trunk/Lib/test/test_support.py", line 1038, in _run_suite
    raise TestFailed(err)
test.test_support.TestFailed: Traceback (most recent call last):
  File "Lib/test/test_socket.py", line 591, in testGetaddrinfo
    self.assertEqual(socktype, socket.SOCK_STREAM)
AssertionError: 2 != 1
msg107996 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2010-06-17 08:43
> But why is that an attribute of a socket object?

Please pretend I did not write this. ;)


Anyway, getaddrinfo() on FreeBSD/Qemu gives this:

 >>> socket.getaddrinfo('localhost', 21)
[(2, 2, 17, '', ('127.0.0.1', 21)), (2, 1, 6, '', ('127.0.0.1', 21)), (2, 5, 132, '', ('127.0.0.1', 21)), (28, 2, 17, '', ('::1', 21, 0, 0)), (28, 1, 6, '', ('::1', 21, 0, 0)), (28, 5, 132, '', ('::1', 21, 0, 0))]
>>> socket.getaddrinfo('localhost', 22)
[(2, 2, 17, '', ('127.0.0.1', 22)), (2, 1, 6, '', ('127.0.0.1', 22)), (2, 5, 132, '', ('127.0.0.1', 22)), (28, 2, 17, '', ('::1', 22, 0, 0)), (28, 1, 6, '', ('::1', 22, 0, 0)), (28, 5, 132, '', ('::1', 22, 0, 0))]
>>> socket.getaddrinfo('localhost', 80)
[(2, 2, 17, '', ('127.0.0.1', 80)), (2, 1, 6, '', ('127.0.0.1', 80)), (2, 5, 132, '', ('127.0.0.1', 80)), (28, 2, 17, '', ('::1', 80, 0, 0)), (28, 1, 6, '', ('::1', 80, 0, 0)), (28, 5, 132, '', ('::1', 80, 0, 0))]
msg108022 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-17 14:35
> Anyway, getaddrinfo() on FreeBSD/Qemu gives this

It seems SOCK_DGRAM is always returned which, as far as I know, doesn't make sense with FTP and SSH protocols. 
At this point, assuming getaddrinfo() correctly binds the original C function, I'd be for just removing this test since it's unreliable across all platforms.
New patch is in attachment.
msg113681 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-12 14:50
I've ported the patch to py3k and checked it works under Mandriva Linux and Windows 7 x64.
msg113894 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-14 13:42
I'm not sure whether it's a problem with my python installation but this is what I get on FreeBSD 7.0 with both python 2.7 and 3.2:


>>> import socket
>>> socket.getaddrinfo('localhost', 80)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
socket.gaierror: [Errno 9] servname not supported for ai_socktype
>>>
msg113896 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-14 13:43
Also with python 2.5 which is the system default.
msg113906 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-14 15:13
This seems to be related with issue 1282647.
Modified patch which skips the test in case of buggy libc version is in attachment.
msg113911 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-14 17:05
Committed in r84024.
msg113957 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-08-15 13:15
test_socket fails on OS X:

======================================================================
ERROR: testGetaddrinfo (test.test_socket.GeneralModuleTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/test/test_socket.py", line 611, in testGetaddrinfo
    socket.getaddrinfo(HOST, None, 0, 0, socket.AI_CANONNAME)
socket.gaierror: [Errno 12] Bad hints

----------------------------------------------------------------------

http://www.python.org/dev/buildbot/all/builders/x86%20Tiger%203.x/builds/791
msg113970 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-15 15:23
Is there someone who can take a look at this on OSX (Ronald?)?
msg114005 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2010-08-15 19:47
I think there is a missing '0' in the failing line:

Index: Lib/test/test_socket.py
===================================================================
--- Lib/test/test_socket.py	(revision 84079)
+++ Lib/test/test_socket.py	(working copy)
@@ -608,7 +608,7 @@
         for _, socktype, _, _, _ in infos:
             self.assertEqual(socktype, socket.SOCK_STREAM)
         # test proto and flags arguments
-        socket.getaddrinfo(HOST, None, 0, 0, socket.AI_CANONNAME)
+        socket.getaddrinfo(HOST, None, 0, 0, 0, socket.AI_CANONNAME)
         socket.getaddrinfo(HOST, None, 0, 0, 0, socket.AI_PASSIVE)
         # a server willing to support both IPv4 and IPv6 will
         # usually do this



With this patch the tests pass, without the patch AI_CANNAME gets used as the value for 'proto'.
msg114036 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-16 05:12
Thanks for the catch. Committed in r84089.
I replaced it with socket.getaddrinfo(HOST, None, 0, 0, socket.SOL_TCP) though, so that the proto argument is tested.
msg114037 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2010-08-16 05:44
There is one thing I don't understand about these tests: they basicly only seem to test if the function implements the right interface (argument counts and the structure of the return value).

Shouldn't the tests also test if the behaviour is somewhat sane? One example of this: test the 'family' flag by looking for www.google.com with both AF_INET and AF_INET6 and then check that the first returns IPv4 information and the second IPv6.
msg114047 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-16 15:05
I think the main problem here is that it's not easy to write reliable tests that work across all platforms (getaddrinfo(host, 'ftp') returning UDP addresses on FreeBSD/Qemu only is an example).
We might try to go a little deeper as you suggested but it's very likely that some buildbots turn red, IMHO.

> One example of this: test the 'family' flag by looking for 
> www.google.com with both AF_INET and AF_INET6 and then check that the 
> first returns IPv4 information and the second IPv6.

This is already done, although for IPv4 only and by using localhost instead of google.com.

        infos = socket.getaddrinfo(HOST, None, socket.AF_INET)
        for family, _, _, _, _ in infos:
            self.assertEqual(family, socket.AF_INET)

I'm attaching a new patch which adds some additional checks.
Feel free to extend it, if you want.
msg114073 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-08-16 20:25
Using www.google.com for IPv6 lookups is a bad idea: whether or not you get an IPv6 address depends on whether your nameserver participates in "google over ipv6". www.python.org would be a better choice.
msg114077 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-08-16 20:56
> Using www.google.com for IPv6 lookups is a bad idea (...)

There is also ipv6.google.com
History
Date User Action Args
2010-08-16 20:56:42vstinnersetnosy: + vstinner
messages: + msg114077
2010-08-16 20:25:46loewissetmessages: + msg114073
2010-08-16 15:05:25giampaolo.rodolasetfiles: + getaddrtests.patch

messages: + msg114047
2010-08-16 05:44:31ronaldoussorensetmessages: + msg114037
2010-08-16 05:12:38giampaolo.rodolasetstatus: open -> closed

messages: + msg114036
2010-08-15 19:47:16ronaldoussorensetmessages: + msg114005
2010-08-15 15:23:28giampaolo.rodolasetmessages: + msg113970
2010-08-15 15:02:40floxsetnosy: + ronaldoussoren
2010-08-15 13:15:43floxsetstatus: closed -> open

nosy: + flox
messages: + msg113957

keywords: + buildbot
2010-08-14 17:05:43giampaolo.rodolasetstatus: open -> closed
resolution: fixed
messages: + msg113911
2010-08-14 15:13:11giampaolo.rodolasetfiles: + getaddrinfo3.patch

messages: + msg113906
2010-08-14 13:43:31giampaolo.rodolasetmessages: + msg113896
2010-08-14 13:42:16giampaolo.rodolasetmessages: + msg113894
2010-08-12 14:50:52pitrousetfiles: + getaddrinfo3.patch

messages: + msg113681
versions: - Python 2.6
2010-06-17 14:36:13giampaolo.rodolasetfiles: - getaddrinfotest.patch
2010-06-17 14:36:10giampaolo.rodolasetfiles: - getaddrinfotest.patch
2010-06-17 14:36:00giampaolo.rodolasetfiles: + getaddrinfo.patch

messages: + msg108022
2010-06-17 08:43:49skrahsetmessages: + msg107996
2010-06-17 08:10:28skrahsetmessages: + msg107994
2010-06-17 00:32:51giampaolo.rodolasetfiles: + getaddrinfotest.patch

messages: + msg107969
2010-06-17 00:22:16giampaolo.rodolasetfiles: + getaddrinfotest.patch
2010-06-17 00:22:08giampaolo.rodolasetfiles: - getaddrinfotest.patch
2010-06-17 00:16:19giampaolo.rodolasetfiles: - getaddrinfotest.patch
2010-06-17 00:16:10giampaolo.rodolasetfiles: + getaddrinfotest.patch

messages: + msg107968
2010-06-14 20:25:29skrahsetnosy: + skrah
messages: + msg107810
2010-06-12 19:18:28giampaolo.rodolasetmessages: + msg107684
2010-06-12 17:49:45pitrousetnosy: + loewis
messages: + msg107677
2010-06-01 20:42:38giampaolo.rodolasetfiles: + getaddrinfotest.patch

messages: + msg106860
2010-05-31 17:07:08pitrousetmessages: + msg106795
2010-05-31 16:45:43exarkunsetmessages: + msg106794
2010-05-30 18:04:37pitrousetfiles: + doc-getaddrinfo.patch
2010-05-30 18:04:31pitrousetfiles: - doc-getaddrinfo.patch
2010-05-30 18:01:46pitrousetfiles: + doc-getaddrinfo.patch
keywords: + patch
messages: + msg106771
2010-05-30 17:23:16pitrousetmessages: + msg106770
2010-05-30 16:58:26pitrousetnosy: + exarkun, mark.dickinson, giampaolo.rodola
2010-05-30 16:58:10pitroucreate