Opened 4 years ago

Last modified 4 years ago

#6401 defect new

twisted.web.test.requesthelper.DummyChannel.TCP does not have the unregisterProducer method.

Reported by: moijes12 Owned by: Kai Zhang
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight, moijes12, Kai Zhang Branch: branches/replace-DummyChannel.TCP-with-StringTransport-6401
branch-diff, diff-cov, branch-cov, buildbot
Author: tomprince

Description

twisted.web.test.requesthelper.DummyChannel.TCP is written for helping in testing of http.Request. It does have a counterpart to the twisted.web.http.Request.registerProducer but not the unregisterProducer. This unregisterProducer needs to be implemented.

Attachments (2)

6401.patch (10.3 KB) - added by Kai Zhang 4 years ago.
Now twisted.web.test.requesthelper.DummyChannel.TCP is inherit from twisted.test.proto_helpers.StringTransport. Tests in twisted.web.test are revised accordingly.
6401.1.patch (13.2 KB) - added by Kai Zhang 4 years ago.
Now twisted.web.test.requesthelper.DummyChannel.TCP is inherit from twisted.test.proto_helpers.StringTransport. Tests in twisted.web.test are revised accordingly.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by DefaultCC Plugin

comment:2 Changed 4 years ago by moijes12

Cc: moijes12 added

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

Rather than expanding this test double, it might be better to consolidate and make the Twisted Web tests work with twisted.test.proto_helpers.StringTransport instead.

comment:4 Changed 4 years ago by Kai Zhang

Owner: set to Kai Zhang

Changed 4 years ago by Kai Zhang

Attachment: 6401.patch added

Now twisted.web.test.requesthelper.DummyChannel.TCP is inherit from twisted.test.proto_helpers.StringTransport. Tests in twisted.web.test are revised accordingly.

comment:5 Changed 4 years ago by Kai Zhang

Cc: Kai Zhang added
Keywords: review added
Owner: Kai Zhang deleted

Now twisted.web.test.requesthelper.DummyChannel.TCP is inherit from twisted.test.proto_helpers.StringTransport. The reason why not use twisted.test.proto_helpers.StringTransport directly is that twisted.web.test.requesthelper.DummyChannel.TCP can set port and return host accordingly.

Now twisted.web.test.requesthelper.DummyChannel.TCP is like this:

class DummyChannel:
     class TCP(StringTransport):
         port = 80
 
         def getHost(self):
             return IPv4Address("TCP", '10.0.0.1', self.port)
     ...

Then I revised the tests in twisted.web.test accordingly.

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

Thanks.

The reason why not use twisted.test.proto_helpers.StringTransport directly is that twisted.web.test.requesthelper.DummyChannel.TCP can set port and return host accordingly

Actually, StringTransport supports this: you can pass the hostAddress keyword argument to its initializer to specify any return value for getHost that you like.

Changed 4 years ago by Kai Zhang

Attachment: 6401.1.patch added

Now twisted.web.test.requesthelper.DummyChannel.TCP is inherit from twisted.test.proto_helpers.StringTransport. Tests in twisted.web.test are revised accordingly.

comment:7 in reply to:  6 Changed 4 years ago by Kai Zhang

Replying to exarkun:

Thanks.

The reason why not use twisted.test.proto_helpers.StringTransport directly is that twisted.web.test.requesthelper.DummyChannel.TCP can set port and return host accordingly

Actually, StringTransport supports this: you can pass the hostAddress keyword argument to its initializer to specify any return value for getHost that you like.

Hi, now twisted.web.test.requesthelper.DummyChannel.TCP is just twisted.test.proto_helpers.StringTransport.

comment:8 Changed 4 years ago by Tom Prince

Author: tomprince
Branch: branches/replace-DummyChannel.TCP-with-StringTransport-6401

(In [38791]) Branching to replace-DummyChannel.TCP-with-StringTransport-6401.

comment:9 Changed 4 years ago by Tom Prince

(In [38792]) Apply 6401.1.patch from kkpattern.

Refs: #6401

comment:10 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to Kai Zhang
  1. Is there a reason for TCP and SSL to be seperate classes, rather than just using StringTransport directly?
    • I did some experiments, and it appears that TCP can entirely be replaced
    • The code does depend on SSL implementing ISSLTransport (although SSL doesn't have all the required methods).
  • So, I think that TCP should just be removed entirely, and SSL can stay as-is, for the moment.
  1. Rather than setting hostAddr directly, you should pass the address to the constructor, instead.

Please resubmit for review after addressing the above two points.

Note: See TracTickets for help on using tickets.