Opened 3 years ago

Closed 21 months ago

#5578 defect closed fixed (fixed)

Some TCP-specific code leaked into twisted/internet/test/connectionmixins.py

Reported by: exarkun Owned by: therve
Priority: high Milestone:
Component: core Keywords: tests
Cc: Branch: branches/stream-client-tests-5578-2
(diff, github, buildbot, log)
Author: therve Launchpad Bug:

Description

twisted/internet/test/connectionmixins.py is intended to hold code useful for any connection-oriented ports/transports. The tests it defines should all be applicable to any kind of SOCK_STREAM sockets. In particular, that means they should run on all of TCP, SSL, and UNIX.

TCPClientTestsMixin is limited to TCP (as you might expect from the name). It needs to be generalized to apply to TCP, SSL, and UNIX.

Change History (6)

comment:1 Changed 2 years ago by therve

  • Owner set to therve

comment:2 Changed 2 years ago by therve

  • Author set to therve
  • Branch set to branches/stream-client-tests-5578

(In [34977]) Branching to 'stream-client-tests-5578'

comment:3 Changed 21 months ago by therve

  • Branch changed from branches/stream-client-tests-5578 to branches/stream-client-tests-5578-2

(In [37460]) Branching to 'stream-client-tests-5578-2'

comment:4 Changed 21 months ago by therve

  • Keywords review added
  • Owner therve deleted

Alright, it looks TCP-free. I used the new mixin in Unix tests, not in SSL yet. I think we can do that in a next branch if that's OK.

comment:5 Changed 21 months ago by tom.prince

  • Keywords review removed
  • Owner set to therve
  1. test_addresses
    • I wouldn't wrap the L{} even if it goes over the limit.
    • You are also losing the verification of the repr.
  2. UnixClientTestsBuilder
    • This should have a test of the repr, as well.
  3. There are a number of twistedchecker failures. But I'm not sure if any of them are worth fixing. I think perhaps putting listen/connect (raising NotImplementedError) in the base class, and putting docs (with epytext markup) might be the right thing for some of them.

Please merge after dealing with repr and optionally the other points.

comment:6 Changed 21 months ago by therve

  • Resolution set to fixed
  • Status changed from new to closed

(In [37572]) Merge stream-client-tests-5578-2

Author: therve
Reviewer: tom.prince
Fixes: #5578

Remove TCP specific bits in the connectionmixins test module, and reuse it in
test_unix.

Note: See TracTickets for help on using tickets.