classification
Title: asyncore test suite
Type: Stage: resolved
Components: Tests Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: giampaolo.rodola, josiah.carlson, josiahcarlson, loewis, michael.foord, pitrou, r.david.murray
Priority: normal Keywords: patch

Created on 2010-04-21 19:33 by giampaolo.rodola, last changed 2010-05-10 15:45 by giampaolo.rodola. This issue is now closed.

Files
File name Uploaded Description Edit
asyncore.patch giampaolo.rodola, 2010-04-21 19:33
asyncore.patch giampaolo.rodola, 2010-04-22 16:14
asyncore.patch giampaolo.rodola, 2010-05-08 11:15
Messages (7)
msg103896 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-04-21 19:33
The patch in attachment provides actual tests for asyncore.dispatcher class API, including all handle_* callback methods.
msg103936 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-04-22 01:20
Thanks for writing these.

The patch looks good to me (caveat: I only have a passing familiarity with asyncore).  A couple of English nits:

   A server which listen on an address and dispatches the 

should be 'listens'

    # EADDRINUSE indicates the socket was correctly binded

should be 'bound'

And one style nit: I'd perfer to see a 'BaseTestAPI' that has no base class, and two TestCase subclasses that use BaseTestAPI as a mixin, one for use_poll=True and one for use_poll=False.  It's not a big deal, though; what you have is fine if you don't feel like changing it.

As discussed on #python-dev, it would be best to make a branch in which to apply this so you can run it on all the buildbots.  This is especially true since you have one test your comments indicate you are pretty sure is not going to work on all platforms, and it would be nice to add skips for those platforms before checking it in to trunk.
msg103945 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-22 10:48
Some remarks:
- I agree with RDM's comment regarding inheritance
- you don't use use_poll in loop_waiting_for_flag()
- calling time.sleep() in loop_waiting_for_flag() looks wrong to me
- test_handle_error should check that the exception state is the expected one (ZeroDivisionError), if at all possible
- in test_set_reuse_addr, if SO_REUSEADDR fails I would suggest skipping the test with self.skip(...), rather than passing silently
- in test_set_reuse_addr, sock.close() rather than sock.close
- does asyncore support IPv6? test_create_socket checks only AF_INET
- do you plan to also test UDP? :)
msg103972 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-04-22 16:14
> should be 'listens'
[...]
> should be 'bound'

done

> I'd prefer to see a 'BaseTestAPI' that has no base class, and two 
> TestCase subclasses that use BaseTestAPI as a mixin, one for 
> use_poll=True and one for use_poll=False.

done

> - you don't use use_poll in loop_waiting_for_flag()

whoops! thanks

> - calling time.sleep() in loop_waiting_for_flag() looks wrong to me

time.sleep() in that loop grants that the condition is retried for at least 1 second before failing the test. By removing time.sleep() a test which is supposed to fail will fail almost immediately.

> - test_handle_error should check that the exception state is the 
> expected one (ZeroDivisionError), if at all possible

Good advice, done.

> - in test_set_reuse_addr, if SO_REUSEADDR fails I would suggest skipping the test with self.skip(...),

done

> in test_set_reuse_addr, sock.close() rather than sock.close

whoops =)

> does asyncore support IPv6? test_create_socket checks only AF_INET

I don't think it would worth it. create_socket() is just a thin wrapper around socket.socket(). Actually I wasn't even sure to add that test or not.

> - do you plan to also test UDP? :)

Unfortunately asyncore does not support UDP.
At a first look it would seems so, since create_socket() accepts two parameters, family and *type*:

    def create_socket(self, family, type):
        sock = socket.socket(family, type)

...but actually we can only use SOCK_STREAM, as the API just takes care of wrapping send() and recv() calls, not recvfrom() and sendto().
msg104079 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-04-24 12:44
Builbots are all ok except Solaris which makes me think that maybe asyncore is broken on such platform:
http://python.org/dev/buildbot/builders/sparc%20solaris10%20gcc%20trunk/builds/728/steps/test/logs/stdio
I've tried to adjust the tests a little bit in r80415 but that didn't work.
I'm not sure how to proceed here.
If I'd have a chance to gain SSH access over the Solaris box I could try to debug the problem, otherwhise we could just disable the failing tests for Solaris.
msg105279 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-08 11:15
I have adjusted the failing tests in r80895 and r80930 which now pass on Solaris. 
It seems Solaris has two problems:

 - OOB data (aka handle_expt) doesn't work, but I'm not too worried about this as it's very rarely used. In past there have been proposals to remove its support, so I'd be for leaving it as it is.

- connect() connects immediately, even before starting the poller.
Now, this may be a problem when writing clients which implement some logic in their handle_connect() method.
Since handle_connect() gets called immediately if the user tries to, say, send some data over the socket, the dispatcher will immediately be closed as the socket is actually still disconnected.

I'd be for starting to commit the patch in attachment which skips such tests on SunOS, then if someone could give me SSH access over the Solaris box used for buildbots I could try to debug the connect() problem and see if I can fix it.
msg105444 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-10 15:44
Committed in r81043 (trunk) and r81044 (3.2).
Thanks for your comments.
History
Date User Action Args
2010-05-10 15:45:07giampaolo.rodolasetstatus: open -> closed
keywords: patch, patch
2010-05-10 15:44:09giampaolo.rodolasetassignee: josiahcarlson -> giampaolo.rodola
components: + Tests
versions: + Python 2.7, Python 3.2
keywords: patch, patch
nosy: loewis, josiahcarlson, pitrou, giampaolo.rodola, josiah.carlson, r.david.murray, michael.foord
messages: + msg105444
resolution: fixed
stage: resolved
2010-05-08 11:15:24giampaolo.rodolasetkeywords: patch, patch
files: + asyncore.patch
messages: + msg105279
2010-04-24 12:45:00giampaolo.rodolasetkeywords: patch, patch
nosy: + loewis
messages: + msg104079

2010-04-22 16:14:13giampaolo.rodolasetkeywords: patch, patch
files: + asyncore.patch
messages: + msg103972
2010-04-22 10:48:18pitrousetkeywords: patch, patch
nosy: + pitrou
messages: + msg103945

2010-04-22 01:20:54r.david.murraysetkeywords: patch, patch

messages: + msg103936
2010-04-21 19:33:59giampaolo.rodolacreate