Opened 4 years ago

Last modified 4 years ago

#6135 defect new

twisted.web.http.HTTPFactory is incompletely tested

Reported by: Jean-Paul Calderone Owned by: Kai Zhang
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight, Kai Zhang Branch:
Author:

Description

A fair portion of the code in this class is not tested by anything in twisted.web.test, and even more of it is not tested directly by twisted.web.test.test_http. It should be (found while porting http.py to Python 3).

Attachments (1)

6135.patch (4.5 KB) - added by Kai Zhang 4 years ago.
Add tests for twisted.web.http.HTTPFactory.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by DefaultCC Plugin

Cc: jknight added

comment:2 Changed 4 years ago by Kai Zhang

Cc: Kai Zhang added
Owner: set to Kai Zhang

Changed 4 years ago by Kai Zhang

Attachment: 6135.patch added

Add tests for twisted.web.http.HTTPFactory.

comment:3 Changed 4 years ago by Kai Zhang

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

I am still new to Twisted. There must be many mistakes in this patch. Please tell me and I will try my best to correct them.

comment:4 Changed 4 years ago by Kai Zhang

Cc: Kai Zhang added

comment:5 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to Kai Zhang
  1. FakeFile: This should be moved somewhere common. (see [ticket:6064#comment:7
  2. There are a number of things that aren't directly coverred: .log, and the code for updating the log time for a start. (There may be some code that executes them, but nothing that tests their behavior).
    • For testing the time update, you probably want to look at twisted.internet.task.Clock, and you'll need to change the code to use a specified clock, rather than the global reactor.
    • You should test overriding _openLogFile, since that is explicitly documented as supported. (I'd suggest, other than for a test of _openLogFile itself, overriding it, instead of patching open)
  3. Use twisted.trial.unittest.TestCase.patch.
  4. The docstring of test_buildProtocol isn't clear about what it expects, in terms of timeOut.

Please resubmit for review after addressing the above points.

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

I wonder how FakeFile is superior to StringIO.StringIO? I can see one way that StringIO.StringIO is better - it raises ValueError from write if it is closed, like an actual file object.

Note: See TracTickets for help on using tickets.