Opened 4 years ago

Closed 4 years ago

#6005 enhancement closed fixed (fixed)

Port twisted.internet.test.test_fdset to Python 3

Reported by: Itamar Turner-Trauring Owned by: Itamar Turner-Trauring
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/fdset-py3-6005-2
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst

Description

twisted.internet.test.test_fdset is a good starting point for testing the networking parts of the reactor; it should be ported to Python 3.

Change History (10)

comment:1 Changed 4 years ago by itamarst

Author: itamarst
Branch: branches/fdset-py3-6005

(In [35741]) Branching to 'fdset-py3-6005'

comment:2 Changed 4 years ago by itamarst

Branch: branches/fdset-py3-6005branches/fdset-py3-6005-2

(In [35793]) Branching to 'fdset-py3-6005-2'

comment:3 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review added
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone

comment:4 Changed 4 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring

Thanks. A few minor points:

  1. I don't like the new timeout=5 in test_fdset.py. Does the test hang instead of failing in a timely manner? Perhaps that could be fixed. Is that a pre-existing defect or a result of the Python 3 port? Maybe we can file a ticket and ignore it for now, since the test passes now.
  2. RemovingDescriptor should probably close read and write in connectionLost instead of doRead.
  3. Some changes to _glibbase.py but some glib-related reactors are skipped when it comes to test_fdset. intentional?

Please merge when the above are addressed.

comment:5 Changed 4 years ago by Itamar Turner-Trauring

  1. Filed #6017, removed 5 second timeout.
  2. Added explanation of why it needs to be done in doRead, plus bonus test from when I was figuring out why.
  3. I think I did this before I made skipping work in run-python3-tests. It's not longer necessary, so I reverted it.

comment:6 Changed 4 years ago by itamarst

Resolution: fixed
Status: newclosed

(In [35808]) Merge fdset-py3-6005-2.

Author: itamar Review: exarkun Fixes: #6005

twisted.internet.test.test_fdset now runs on Python 3; this means the reactor now kinda sorta works on Python 3.

comment:7 Changed 4 years ago by itamarst

Resolution: fixed
Status: closedreopened

(In [35812]) Revert fdset-py3-6005-2: Failing OS X tests.

Reopens: #6005.

comment:8 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review added
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone
Status: reopenednew

comment:9 Changed 4 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring

I guess that seems okay. Perhaps not using FileDescriptor at all in the new test would be better, though, if you feel like it.

comment:10 Changed 4 years ago by itamarst

Resolution: fixed
Status: newclosed

(In [35821]) Merge fdset-py3-6005-2, this time for reals.

Author: itamar Review: exarkun Fixes: #6005

test_fdset works on Python 3, which probably means the reactor kinda sorta works on Python 3.

Note: See TracTickets for help on using tickets.