Ticket #1781 enhancement closed wontfix

Opened 8 years ago

Last modified 6 years ago

Remove TestCase._wait from Trial

Reported by: jml Owned by:
Priority: high Milestone:
Component: trial Keywords:
Cc: spiv, exarkun Branch:
Author: Launchpad Bug:

Description

Trial starts and stops the reactor. This is not good. It should just set up a bunch of deferreds and run the reactor once, stopping only when complete.

Change History

1

Changed 8 years ago by jml

  • status changed from new to assigned

2

Changed 8 years ago by jml

  • priority changed from normal to highest

3

Changed 8 years ago by jml

  • cc spiv added

So, here's where I'm at

  1. This branch can't be merged til #1883 gets merged.
  2. Everything is being run inside exactly one _wait call
  3. A whole stack of Trial's tests need to be changed so we aren't re-entering the reactor

Once 3 gets done, I'll ditch wait and run everything inside a reactor.run / reactor.stop pair.

4

Changed 8 years ago by jml

2 and 3 are done.

I've changed my mind about 1.

spiv, I'd appreciate it if you could cast your eye over the branch (source:branches/deferred-1781-2)

5

Changed 8 years ago by jml

  • cc exarkun added

Running on full-2.4-debian64 gave the following errors:

===============================================================================
[ERROR]: twisted.mail.test.test_pop3.AnotherPOP3TestCase.testAuthListing

Traceback (most recent call last):
  File "/home/buildslave/slaves/twisted/full2.4-debian64/Twisted/twisted/internet/base.py", line 532, in runUntilCurrent
    f(*a, **kw)
  File "/home/buildslave/slaves/twisted/full2.4-debian64/Twisted/twisted/internet/process.py", line 44, in reapAllProcesses
    process.reapProcess()
  File "/home/buildslave/slaves/twisted/full2.4-debian64/Twisted/twisted/internet/process.py", line 556, in reapProcess
    log.msg('Failed to reap %d:' % self.pid)
exceptions.TypeError: int argument required
===============================================================================
[ERROR]: twisted.test.test_internet.SystemEventTestCase.testTriggerSystemEvent1

Traceback (most recent call last):
  File "/home/buildslave/slaves/twisted/full2.4-debian64/Twisted/twisted/internet/base.py", line 532, in runUntilCurrent
    f(*a, **kw)
  File "/home/buildslave/slaves/twisted/full2.4-debian64/Twisted/twisted/internet/process.py", line 44, in reapAllProcesses
    process.reapProcess()
  File "/home/buildslave/slaves/twisted/full2.4-debian64/Twisted/twisted/internet/process.py", line 556, in reapProcess
    log.msg('Failed to reap %d:' % self.pid)
exceptions.TypeError: int argument required
-------------------------------------------------------------------------------

I assume this has to do with the changed clean up code.

6

Changed 8 years ago by exarkun

I'd like to understand those tracebacks before this branch is merged. Fixing them in the obvious way may well just be papering over a real bug, either existing or introduced in this branch.

7

Changed 8 years ago by jml

Likewise. Although, I don't even know what the obvious way of fixing them is :)

They are race conditions, it seems.

8

Changed 8 years ago by jml

OK. More info.

2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ProcessAliasTestCase.testAliasResolution <--
2006/07/16 22:46 EST [-] Process constructed: echo, ['echo'], {}, None
2006/07/16 22:46 EST [-] Process constructed: echo, ['echo'], {}, None
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ProcessAliasTestCase.testCyclicAlias <--
2006/07/16 22:46 EST [-] Process constructed: echo, ['echo'], {}, None
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ProcessAliasTestCase.testProcessAlias <--
2006/07/16 22:46 EST [-] Process constructed: /Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/mail/test/process.alias.sh, ['/Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/mail/test/process.alias.sh'], {}, None
2006/07/16 22:46 EST [-] /Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/mail/alias.py:214: exceptions.DeprecationWarning: Deferred.setTimeout is deprecated.  Look for timeout support specific to the API you are using instead.
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.RelayTestCase.testExists <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.RelayerTestCase.testMailData <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.RelayerTestCase.testMailFrom <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.RelayerTestCase.testMailTo <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ServiceDomainTestCase.testReceivedHeader <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ServiceDomainTestCase.testValidateFrom <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ServiceDomainTestCase.testValidateTo <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ServiceDomainTestCase.testValidateToBadDomain <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.ServiceDomainTestCase.testValidateToBadUsername <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.VirtualPOP3TestCase.testAuthenticateAPOP <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.VirtualPOP3TestCase.testAuthenticateBadPasswordPASS <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.VirtualPOP3TestCase.testAuthenticateBadUserPASS <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.VirtualPOP3TestCase.testAuthenticateIncorrectResponseAPOP <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.VirtualPOP3TestCase.testAuthenticateIncorrectUserAPOP <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_mail.VirtualPOP3TestCase.testAuthenticatePASS <--
2006/07/16 22:46 EST [-] --> twisted.mail.test.test_pop3.AnotherPOP3TestCase.testAuthListing <--
2006/07/16 22:46 EST [-] New connection from loopback
2006/07/16 22:46 EST [-] Failed to reap None (None):
2006/07/16 22:46 EST [-] Unhandled Error
	Traceback (most recent call last):
	  File "/Users/jml/projects/Twisted/trunk/twisted/protocols/loopback.py", line 85, in loopback
	    reactor.iterate(0.01) # this is to clear any deferreds
	  File "/Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/internet/base.py", line 381, in iterate
	    self.runUntilCurrent()
	  File "/Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/internet/base.py", line 532, in runUntilCurrent
	    f(*a, **kw)
	  File "/Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/internet/process.py", line 44, in reapAllProcesses
	    process.reapProcess()
	--- <exception caught here> ---
	  File "/Users/jml/Projects/Twisted/branches/deferred-1781-2/twisted/internet/process.py", line 558, in reapProcess
	    pid, status = os.waitpid(self.pid, os.WNOHANG)
	exceptions.TypeError: an integer is required

9

Changed 8 years ago by jml

So, it looks like ProcessAliasTestCase doesn't clean up its processes. Then, testAuthListing uses loopback, which runs reactor.iterate, which triggers a reapProcess call.

How do I proceed from here?

10

Changed 8 years ago by jml

  1. Fix loopback to not call reactor.iterate. This would mean making it return a Deferred, which would mean changing a 'lot' of tests.
  2. Insert an arbitrary d = defer.Deferred(); reactor.callLater(0, d.callback, None); return d into TestCase._cbCleanup.
  3. Do something to correctly clean up whatever processes are misbehaving.

I reckon 1.

11

Changed 8 years ago by jml

  • milestone set to Twisted-2.5

12

Changed 8 years ago by jml

OK, touching this bug after a long break.

#1936 added a new loopbackAsync and deprecated the old, blocking loopback. It meant changing a lot of tests.

There are still a lot of unclean tests that become failing tests when I drop the iterate() calls in cleanup. I propose implementing something similar to what foom suggests in #1883 and giving a warning for unclean tests.

13

Changed 8 years ago by jml

  • milestone Twisted-2.5 deleted

We want this in Twisted 2.7

14

Changed 7 years ago by jml

  • priority changed from highest to high

15

Changed 6 years ago by jml

  • status changed from assigned to closed
  • resolution set to wontfix

It would be really, really good if all of the tests could be run in one never-restarted reactor. It would also be really, really good if Trial provided a way to run a suite of tests in one reactor.

However:

  • This is not the best way to remove the _wait ugliness from Trial
  • The current branch is way too big and conflicty.

I have filed #2900, which supersedes this ticket.

16

Changed 3 years ago by <automation>

  • owner jml deleted
Note: See TracTickets for help on using tickets.