Opened 10 years ago

Closed 10 years ago

#4251 defect closed fixed (fixed)

StringTransport's disconnecting instance variable is initialized to zero, not False, contrary to the documentation

Reported by: jesstess Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: jesstess Branch: branches/stringtransport-disconnecting-4251
branch-diff, diff-cov, branch-cov, buildbot
Author: jesstess

Description


Attachments (1)

proto_helpers_false.patch (385 bytes) - added by jesstess 10 years ago.

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by jesstess

Attachment: proto_helpers_false.patch added

comment:1 Changed 10 years ago by jesstess

Keywords: review added
Owner: jesstess deleted

Since there's no functional change except to the str() and repr() of disconnecting, can I get away with no unit test and just get a review and apply this on trunk?

comment:2 Changed 10 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to jesstess

It looks like there's current no test coverage for the disconnecting attribute at all. How about adding one that verifies it starts as a false value and becomes true when loseConnection is called.

Presumably the test would pass before the proposed change, so it's not like this is going to prove that False is vastly superior to 0, but as long as someone is looking at this code, we may as well improve test coverage a bit. :)

comment:3 Changed 10 years ago by jesstess

Author: jesstess
Branch: branches/stringtransport-disconnecting-4251

(In [28123]) Branching to 'stringtransport-disconnecting-4251'

comment:4 Changed 10 years ago by jesstess

(In [28124]) Add a docstring, unit test, and topfile.

refs #4251

comment:5 Changed 10 years ago by jesstess

Keywords: review added
Owner: jesstess deleted

comment:6 Changed 10 years ago by Michael Hudson-Doyle

Keywords: review removed

I think using assertIdentical to check that disconnecting really is False, then True would be justified here.

Please merge after making that change.

comment:7 Changed 10 years ago by jesstess

(In [28127]) Use assertFalse and assertTrue.

refs #4251

comment:8 Changed 10 years ago by jesstess

Resolution: fixed
Status: newclosed

(In [28128]) Merge stringtransport-disconnecting-4251

Author: jesstess Reviewers: mwh Fixes: #4251

Initialize the disconnecting instance variable of StringTransport to False instead of 0.

comment:9 Changed 9 years ago by <automation>

Note: See TracTickets for help on using tickets.