Skip to content

Commit

Permalink
fix trial -j / ctrl-c interaction
Browse files Browse the repository at this point in the history
  • Loading branch information
glyph committed Jul 21, 2023
1 parent 949f4b6 commit e33b496
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/twisted/newsfragments/10340.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`trial -j` no longer causes a traceback on exit when stopped with control-C.
1 change: 1 addition & 0 deletions src/twisted/newsfragments/11707.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

32 changes: 26 additions & 6 deletions src/twisted/trial/_dist/disttrial.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,35 @@ async def runAndReport(n: int) -> DistReporter:
await startedPool.join()

def _run(self, test: Union[TestCase, TestSuite], untilFailure: bool) -> IReporter:
result: Union[Failure, DistReporter]
result: Union[Failure, DistReporter, None] = None
reactorStopping: bool = False
testsInProgress: Deferred[object]

def capture(r):
def capture(r: Union[Failure, DistReporter]) -> None:
nonlocal result
result = r

d = Deferred.fromCoroutine(self.runAsync(test, untilFailure))
d.addBoth(capture)
d.addBoth(lambda ignored: self._reactor.stop())
def maybeStopTests() -> None | Deferred[object]:
nonlocal reactorStopping
reactorStopping = True

Check warning on line 454 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L454

Added line #L454 was not covered by tests
if result is None:
testsInProgress.cancel()
return testsInProgress
return None

Check warning on line 458 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L456-L458

Added lines #L456 - L458 were not covered by tests

def maybeStopReactor(result: object) -> object:

Check warning on line 460 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L460

Added line #L460 was not covered by tests
if not reactorStopping:
self._reactor.stop()
return result

Check warning on line 463 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L462-L463

Added lines #L462 - L463 were not covered by tests

self._reactor.addSystemEventTrigger("before", "shutdown", maybeStopTests)

Check warning on line 465 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L465

Added line #L465 was not covered by tests

testsInProgress = (

Check warning on line 467 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L467

Added line #L467 was not covered by tests
Deferred.fromCoroutine(self.runAsync(test, untilFailure))
.addBoth(capture)
.addBoth(maybeStopReactor)
)

self._reactor.run()

if isinstance(result, Failure):
Expand All @@ -458,7 +478,7 @@ def capture(r):
# mypy can't see that raiseException raises an exception so we can
# only get here if result is not a Failure, so tell mypy result is
# certainly a DistReporter at this point.
assert isinstance(result, DistReporter)
assert isinstance(result, DistReporter), f"{result} is not DistReporter"

Check warning on line 481 in src/twisted/trial/_dist/disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/disttrial.py#L481

Added line #L481 was not covered by tests

# Unwrap the DistReporter to give the caller some regular IReporter
# object. DistReporter isn't type annotated correctly so fix it here.
Expand Down
20 changes: 19 additions & 1 deletion src/twisted/trial/_dist/test/test_disttrial.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

from twisted.internet import interfaces
from twisted.internet.base import ReactorBase
from twisted.internet.defer import Deferred, succeed
from twisted.internet.defer import CancelledError, Deferred, succeed
from twisted.internet.error import ProcessDone
from twisted.internet.protocol import ProcessProtocol, Protocol
from twisted.internet.test.modulehelpers import AlternateReactor
Expand Down Expand Up @@ -125,6 +125,12 @@ def stop(self):
See L{IReactorCore.stop}.
"""
MemoryReactorClock.stop(self)
# TODO: implementing this more comprehensively in MemoryReactor would
# be nice, this is rather hard-coded to disttrial's current
# implementation.
if "before" in self.triggers:
if "shutdown" in self.triggers["before"]:
self.triggers["before"]["shutdown"][0][0]()

Check warning on line 133 in src/twisted/trial/_dist/test/test_disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/test/test_disttrial.py#L133

Added line #L133 was not covered by tests
self.stopCount += 1

def run(self):
Expand All @@ -139,6 +145,9 @@ def run(self):

for f, args, kwargs in self.whenRunningHooks:
f(*args, **kwargs)
self.stop()

Check warning on line 148 in src/twisted/trial/_dist/test/test_disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/test/test_disttrial.py#L148

Added line #L148 was not covered by tests
# do not count internal 'stop' against trial-initiated .stop() count
self.stopCount -= 1

Check warning on line 150 in src/twisted/trial/_dist/test/test_disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/test/test_disttrial.py#L150

Added line #L150 was not covered by tests


class CountingReactorTests(SynchronousTestCase):
Expand Down Expand Up @@ -461,6 +470,15 @@ def test_runUnexpectedError(self) -> None:
assert_that(errors, has_length(1))
assert_that(errors[0][1].type, equal_to(WorkerPoolBroken))

def test_runUnexpectedErrorCtrlC(self) -> None:
"""
If the reactor is stopped by C-c (i.e. `run` returns before the test
case's Deferred has been fired) we should cancel the pending test run.
"""
runner = self.getRunner(workerPoolFactory=LocalWorkerPool)
with self.assertRaises(CancelledError):
runner.run(self.suite)

Check warning on line 480 in src/twisted/trial/_dist/test/test_disttrial.py

View check run for this annotation

Codecov / codecov/patch

src/twisted/trial/_dist/test/test_disttrial.py#L478-L480

Added lines #L478 - L480 were not covered by tests

def test_runUnexpectedWorkerError(self) -> None:
"""
If for some reason the worker process cannot run a test, the error is
Expand Down

0 comments on commit e33b496

Please sign in to comment.