Opened 5 years ago

Closed 3 years ago

#3796 defect closed fixed (fixed)

Intermittent failure of twisted.web.test.test_webclient.WebClientSSLTestCase.test_timeoutTriggering

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone:
Component: core Keywords: tests
Cc: Branch: branches/oldwebclient-cleanup-3796
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

On OS X, I saw this fail (while working on #3782):

[ERROR]: twisted.web.test.test_webclient.WebClientSSLTestCase.test_timeoutTriggering
Traceback (most recent call last):
Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean.
Selectables:
<<class 'twisted.internet.tcp.TLSConnection'> to ('127.0.0.1', 59660) at 1345af0>

Change History (5)

comment:1 Changed 3 years ago by <automation>

  • Owner exarkun deleted

comment:2 Changed 3 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/oldwebclient-cleanup-3796

(In [32326]) Branching to 'oldwebclient-cleanup-3796'

comment:3 Changed 3 years ago by exarkun

  • Keywords review added

I changed getPage and downloadPage to only fire their result Deferred after the underlying connection gives the HTTPClient its connectionLost notification.

I tested this change in the #5118 branch (with the icky tearDown added in that branch deleted) on my Ubuntu Karmic box to verify it actually solves the problems encountered there, and it does. As is the branch fails 5 webclient ssl tests there, with this patch it passes them all reliably.

Build results

comment:4 Changed 3 years ago by glyph

  • Keywords review removed
  • Owner set to exarkun

Aah, what a nice, self-contained change. Looks great. Should make naive third-party tests using getPage a bit more reliable, too. (And thanks for the pyflakes fixes.)

No further comments; please land.

comment:5 Changed 3 years ago by exarkun

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

(In [32329]) Merge oldwebclient-cleanup-3796

Author: exarkun
Reviewer: glyph
Fixes: #3796

Change twisted.web.client.getPage and twisted.web.client.downloadPage to not
fire their returned Deferred until the connection used for the request each
issues is closed. This avoids problems in unit tests using these APIs where
trial reports an unclean reactor due to these connections still being up when
the test thinks it is finished.

Note: See TracTickets for help on using tickets.