Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#6521 defect closed fixed (fixed)

StringTransportWithDisconnection.connectionLost should use a failure

Reported by: Adi Roiban Owned by: therve
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/string-transport-disconnection-failure-6521
branch-diff, diff-cov, branch-cov, buildbot
Author: therve


The current implementation of t.test.proto_helpers.StringTransportWithDisconnection.connectionLost is calling self.protocol.connectionLost() with an Exception and not a Failure.

Change History (8)

comment:1 Changed 8 years ago by Adi Roiban

Keywords: review added
Owner: Adi Roiban deleted

I am trying to fix this issue.

To be able to commit my changes I have published them on GitHub.

The diff can be downloaded from here:

I have also created a Pull Request on GitHub in case it is of any help

I have fixed the implementation and also updated the failing tests.

There is still one un-caught exceptions but I don't know how to track it

$ trial twisted.test


Traceback (most recent call last):
Failure: twisted.internet.error.ConnectionDone: Connection was closed cleanly: Bye..

Ran 2074 tests in 30.939s

FAILED (skips=58, expectedFailures=4, errors=1, successes=2011)

$ trial twisted.test.test_pb


    test_sync ...                                                          [OK]

Ran 45 tests in 0.527s

PASSED (successes=45)

Any suggestion about how to look for fixing that exception is much appreciated.


comment:2 Changed 8 years ago by therve

Owner: set to therve

comment:3 Changed 8 years ago by therve

Author: therve
Branch: branches/string-transport-disconnection-failure-6521

(In [38508]) Branching to 'string-transport-disconnection-failure-6521'

comment:4 Changed 8 years ago by therve

Keywords: review removed

Looks good, applying.

comment:5 Changed 8 years ago by therve

Resolution: fixed
Status: newclosed

(In [38517]) Merge string-transport-disconnection-failure-6521

Author: adiroiban Reviewer: therve Fixes: #6521

Pass a Failure instead of a plain exception in StringTransportWithDisconnection.loseConnection.

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

Might have been nice to mention the apparently random other changes in the commit message. I *almost* reverted this change, because it looked like you just had some extra junk lying around in your trunk working copy that got committed by accident.

comment:7 Changed 8 years ago by Adi Roiban

Uh... I forgot about the news file.

Was the flaky test fixed? If yes, how? or that strange behavior only occur on my system.


comment:8 Changed 8 years ago by therve

Exarkun: sorry, I got lost in cleanup maze.

Adiroiban: yeah there was another missing assertFailure. You could track it relatively successfully using trial --force-gc.

Note: See TracTickets for help on using tickets.