Twisted: Ticket #5730: stdio endpoint tests leave a dirty reactorhttps://twistedmatrix.com/trac/ticket/5730
<p>
The problem with many of the tests in <code>StandardIOEndpointsTestCase</code> is that they're using a real reactor, but not actually <a class="ext-link" href="http://mumak.net/stuff/twisted-disconnect.html"><span class="icon">​</span>waiting for the disconnection notification</a>.
</p>
<p>
It would be nice to avoid using a real reactor (or, for that matter, real stdio streams) in most of these tests; if we want an integration test it should probably live in an isolated subprocess.
</p>
<p>
As per <a class="ext-link" href="http://twistedmatrix.com/trac/ticket/4697#comment:18"><span class="icon">​</span>this</a> comment on <a class="closed ticket" href="https://twistedmatrix.com/trac/ticket/4697" title="#4697: enhancement: endpoint: stdio (closed: fixed)">#4697</a>, this needs to be fixed.
</p>
en-usTwistedhttps://twistedmatrix.com/trac/chrome/common/trac_banner.pnghttps://twistedmatrix.com/trac/ticket/5730
Trac 1.2ashfallMon, 20 Aug 2012 03:07:38 GMThttps://twistedmatrix.com/trac/ticket/5730#comment:1
https://twistedmatrix.com/trac/ticket/5730#comment:1
<p>
<a class="closed ticket" href="https://twistedmatrix.com/trac/ticket/5892" title="#5892: defect: dirty reactor errors in test_endpoints (closed: duplicate)">#5892</a> looks like a duplicate of this.
</p>
TicketGlyphWed, 04 Jun 2014 18:34:52 GMTdescription, summary changedhttps://twistedmatrix.com/trac/ticket/5730#comment:2
https://twistedmatrix.com/trac/ticket/5730#comment:2
<ul>
<li><strong>description</strong>
modified (<a href="/trac/ticket/5730?action=diff&amp;version=2">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>Fix the dirty reactor warnings when tests for the stdio endpoints are run on Windows</em> to <em>stdio endpoint tests leave a dirty reactor</em>
</li>
</ul>
<p>
Previously, this ticket indicated that the issue was with Windows. However, when running test_endpoints under Emacs (which has a relatively slow pipe consumer on stdout) I have been seeing this somewhat regularly:
</p>
<pre class="wiki">[ERROR]
Traceback (most recent call last):
Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean.
Selectables:
&lt;twisted.internet.process.ProcessWriter object at 0x104216e10&gt;
twisted.internet.test.test_endpoints.StandardIOEndpointsTestCase.test_protocol
</pre>
TicketGlyphWed, 04 Jun 2014 21:29:24 GMTbranch, branch_author sethttps://twistedmatrix.com/trac/ticket/5730#comment:3
https://twistedmatrix.com/trac/ticket/5730#comment:3
<ul>
<li><strong>branch</strong>
set to <em>branches/dirty-stdio-endpoint-5730</em>
</li>
<li><strong>branch_author</strong>
set to <em>glyph</em>
</li>
</ul>
<p>
(In <a class="missing changeset" title="No changeset 42774 in the repository">[42774]</a>) Branching to dirty-stdio-endpoint-5730.
</p>
TicketGlyphWed, 04 Jun 2014 22:35:02 GMTkeywords set; owner deletedhttps://twistedmatrix.com/trac/ticket/5730#comment:4
https://twistedmatrix.com/trac/ticket/5730#comment:4
<ul>
<li><strong>owner</strong>
<em>ashfall</em> deleted
</li>
<li><strong>keywords</strong>
<em>review</em> added
</li>
</ul>
TicketAdi RoibanFri, 13 Jun 2014 09:53:05 GMTowner set; keywords deletedhttps://twistedmatrix.com/trac/ticket/5730#comment:5
https://twistedmatrix.com/trac/ticket/5730#comment:5
<ul>
<li><strong>owner</strong>
set to <em>Glyph</em>
</li>
<li><strong>keywords</strong>
<em>review</em> removed
</li>
</ul>
<p>
Does this have a news file?
</p>
<hr />
<p>
Is this supposed to work on Python 3, as for PY3 I see that it is set to None?
</p>
<hr />
<p>
with then new setup method I find it hard to read tests...
I read the docstring of test_protocolCreation ... and then read
the code... but it looks like protocol is already created and the
test does not directly create the protocol
...same for test_passedReactor...
</p>
<p>
Maybe this can be merged into a single test
</p>
<hr />
<p>
I think that the patch looks good and the removal of reactor singleton usage is welcomed.
</p>
<p>
Thanks
</p>
TicketGlyphThu, 26 Jun 2014 22:49:38 GMThttps://twistedmatrix.com/trac/ticket/5730#comment:6
https://twistedmatrix.com/trac/ticket/5730#comment:6
<p>
Replying to <a class="ticket" href="https://twistedmatrix.com/trac/ticket/5730#comment:5" title="Comment 5">adiroiban</a>:
</p>
<blockquote class="citation">
<p>
Does this have a news file?
</p>
</blockquote>
<p>
It does now. Just a misc though, this is just a test issue.
</p>
<blockquote class="citation">
<p>
Is this supposed to work on Python 3, as for PY3 I see that it is set to None?
</p>
</blockquote>
<p>
There's no change to python 3 support in this branch, as it's just a fix for a test failure.
</p>
<blockquote class="citation">
<p>
with then new setup method I find it hard to read tests...
I read the docstring of test_protocolCreation ... and then read
the code... but it looks like protocol is already created and the
test does not directly create the protocol
...same for test_passedReactor...
</p>
<p>
Maybe this can be merged into a single test
</p>
</blockquote>
<p>
Out of scope for this change :-). Also, tests should be as fine-grained as possible, and I don't see that a lot would be gained by merging them together.
</p>
<blockquote class="citation">
<p>
I think that the patch looks good and the removal of reactor singleton usage is welcomed.
</p>
</blockquote>
<p>
Great.
</p>
<p>
Again, for future reference, please include the next step (please merge, please resubmit) in your reviews.
</p>
<p>
Thanks!
</p>
TicketGlyphThu, 26 Jun 2014 22:55:31 GMTstatus changed; resolution sethttps://twistedmatrix.com/trac/ticket/5730#comment:7
https://twistedmatrix.com/trac/ticket/5730#comment:7
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
(In <a class="missing changeset" title="No changeset 42819 in the repository">[42819]</a>) Merge dirty-stdio-endpoint-5730
</p>
<p>
Author: dreid, glyph
Reviewer: adiroiban
Fixes: <a class="closed ticket" href="https://twistedmatrix.com/trac/ticket/5730" title="#5730: defect: stdio endpoint tests leave a dirty reactor (closed: fixed)">#5730</a>
</p>
<p>
Fix an intermittently failing (dirty reactor) test for StandardIOEndpoint.
</p>
Ticket