Glyph wrote:
> Stefano, this looks like a great contribution: a serious bug fix in a bit of
> core functionality, with a test. Thanks so much!
Thank you! It's a pleasure to have one's code be called by code in
the Twisted framework and to get stuff done with it! And it's an
honor to receive support from such a wonderful community, already
ever so enjoyable and enlightening even when just lurking among it!
> I'll try to do a complete review soon, but my immediate impression is that
> test_disconnectEvent needs a better docstring. The word 'correctly' in a
> docstring is always a clue that the writer gave up and stopped trying to
> describe what 'correct' behavior is :).
Good point, I totally agree with you. "correctly" is most often the
wrong word and a shortcut which leads to unpleasant consequences
once what is "correct" gets better defined later by explicitly
broken behavior.
BTW that line was actually shamelessly copy-pasted from the previous
test's docstring ;P
> if you could try to rephrase it as "When a transport is (...) and then (...),
> (...) should be called on (...)".
>
> (I will have a more concrete suggestion, with fewer "..."s when I have time
> to go through the ticket history and review the patch in detail.)
OK, take two:
This test checks that a protocol gets notified of a lost connection
when its transport has a registered producer writing more than 64KB
to it and is then asked to stopProducing without any more data being
written to it. This ought to prevent that sockets get stuck in
CLOSE_WAIT state when these peculiar conditions occur. See #4719.
Another thing I'm not too sure about is if TCPClientTestsBuilder
("Builder defining tests relating to L{IReactorTCP.connectTCP}.") is
the right class for holding such a test, which is not very related
to connectTCP.
ciao thanks
ste
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
