-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a shutdown_condition to setup #81
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,19 @@ def start(self): | |
self.started = True | ||
|
||
|
||
class FakeFunctionRegistry(object): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for a fake, you can use the real class. |
||
|
||
def __init__(self): | ||
self.atexit = [] | ||
self.has_run = False | ||
|
||
def register(self, func, *args): | ||
self.atexit.append((func, args)) | ||
|
||
def run(self): | ||
self.has_run = True | ||
|
||
|
||
class SetupTests(TestCase): | ||
""" | ||
Tests for setup(). | ||
|
@@ -69,7 +82,7 @@ def test_first_runs_reactor(self): | |
With it first call, setup() runs the reactor in a thread. | ||
""" | ||
reactor = FakeReactor() | ||
EventLoop(lambda: reactor, lambda f, *g: None).setup() | ||
EventLoop(lambda: reactor, FakeFunctionRegistry()).setup() | ||
reactor.started.wait(5) | ||
self.assertNotEqual(reactor.thread_id, None) | ||
self.assertNotEqual(reactor.thread_id, threading.current_thread().ident) | ||
|
@@ -80,7 +93,7 @@ def test_second_does_nothing(self): | |
The second call to setup() does nothing. | ||
""" | ||
reactor = FakeReactor() | ||
s = EventLoop(lambda: reactor, lambda f, *g: None) | ||
s = EventLoop(lambda: reactor, FakeFunctionRegistry()) | ||
s.setup() | ||
s.setup() | ||
reactor.started.wait(5) | ||
|
@@ -91,18 +104,18 @@ def test_stop_on_exit(self): | |
setup() registers an exit handler that stops the reactor, and an exit | ||
handler that logs stashed EventualResults. | ||
""" | ||
atexit = [] | ||
reactor = FakeReactor() | ||
s = EventLoop(lambda: reactor, lambda f, *args: atexit.append((f, args))) | ||
registry = FakeFunctionRegistry() | ||
s = EventLoop(lambda: reactor, registry) | ||
s.setup() | ||
self.assertEqual(len(atexit), 2) | ||
self.assertEqual(len(registry.atexit), 2) | ||
self.assertFalse(reactor.stopping) | ||
f, args = atexit[0] | ||
f, args = registry.atexit[0] | ||
self.assertEqual(f, reactor.callFromThread) | ||
self.assertEqual(args, (reactor.stop,)) | ||
f(*args) | ||
self.assertTrue(reactor.stopping) | ||
f, args = atexit[1] | ||
f, args = registry.atexit[1] | ||
self.assertEqual(f, _store.log_errors) | ||
self.assertEqual(args, ()) | ||
f(*args) # make sure it doesn't throw an exception | ||
|
@@ -132,7 +145,7 @@ def fakeStartLoggingWithObserver(observer, setStdout=1): | |
logging.append(observer) | ||
|
||
reactor = FakeReactor() | ||
loop = EventLoop(lambda: reactor, lambda f, *g: None, | ||
loop = EventLoop(lambda: reactor, FakeFunctionRegistry(), | ||
fakeStartLoggingWithObserver) | ||
loop.setup() | ||
self.assertTrue(logging) | ||
|
@@ -144,7 +157,7 @@ def test_stop_logging_on_exit(self): | |
""" | ||
observers = [] | ||
reactor = FakeReactor() | ||
s = EventLoop(lambda: reactor, lambda f, *arg: None, | ||
s = EventLoop(lambda: reactor, FakeFunctionRegistry(), | ||
lambda observer, setStdout=1: observers.append(observer)) | ||
s.setup() | ||
self.addCleanup(observers[0].stop) | ||
|
@@ -160,7 +173,7 @@ def fakeStartLoggingWithObserver(observer, setStdout=1): | |
self.addCleanup(observer.stop) | ||
original = warnings.showwarning | ||
reactor = FakeReactor() | ||
loop = EventLoop(lambda: reactor, lambda f, *g: None, | ||
loop = EventLoop(lambda: reactor, FakeFunctionRegistry(), | ||
fakeStartLoggingWithObserver) | ||
loop.setup() | ||
self.assertIdentical(warnings.showwarning, original) | ||
|
@@ -169,39 +182,35 @@ def test_start_watchdog_thread(self): | |
""" | ||
setup() starts the shutdown watchdog thread. | ||
""" | ||
thread = FakeThread() | ||
reactor = FakeReactor() | ||
loop = EventLoop(lambda: reactor, lambda *args: None, | ||
watchdog_thread=thread) | ||
loop = EventLoop(lambda: reactor, FakeFunctionRegistry()) | ||
loop.setup() | ||
self.assertTrue(thread.started) | ||
self.assertTrue(loop._watchdog_thread.is_alive) | ||
|
||
def test_no_setup(self): | ||
""" | ||
If called first, no_setup() makes subsequent calls to setup() do | ||
nothing. | ||
""" | ||
observers = [] | ||
atexit = [] | ||
thread = FakeThread() | ||
registry = FakeFunctionRegistry() | ||
reactor = FakeReactor() | ||
loop = EventLoop(lambda: reactor, lambda f, *arg: atexit.append(f), | ||
lambda observer, *a, **kw: observers.append(observer), | ||
watchdog_thread=thread) | ||
loop = EventLoop(lambda: reactor, | ||
registry, | ||
lambda observer, *a, **kw: observers.append(observer)) | ||
|
||
loop.no_setup() | ||
loop.setup() | ||
self.assertFalse(observers) | ||
self.assertFalse(atexit) | ||
self.assertFalse(registry.atexit) | ||
self.assertFalse(reactor.runs) | ||
self.assertFalse(thread.started) | ||
|
||
def test_no_setup_after_setup(self): | ||
""" | ||
If called after setup(), no_setup() throws an exception. | ||
""" | ||
reactor = FakeReactor() | ||
s = EventLoop(lambda: reactor, lambda f, *g: None) | ||
s = EventLoop(lambda: reactor, FakeFunctionRegistry()) | ||
s.setup() | ||
self.assertRaises(RuntimeError, s.no_setup) | ||
|
||
|
@@ -211,7 +220,7 @@ def test_setup_registry_shutdown(self): | |
setup(). | ||
""" | ||
reactor = FakeReactor() | ||
s = EventLoop(lambda: reactor, lambda f, *g: None) | ||
s = EventLoop(lambda: reactor, FakeFunctionRegistry()) | ||
s.setup() | ||
self.assertEqual(reactor.events, | ||
[("before", "shutdown", s._registry.stop)]) | ||
|
@@ -240,7 +249,7 @@ def test_posix(self): | |
""" | ||
reactor = FakeReactor() | ||
reaps = [] | ||
s = EventLoop(lambda: reactor, lambda f, *g: None, | ||
s = EventLoop(lambda: reactor, FakeFunctionRegistry(), | ||
reapAllProcesses=lambda: reaps.append(1)) | ||
s.setup() | ||
reactor.advance(0.1) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,7 @@ | |
|
||
from twisted.trial.unittest import TestCase | ||
|
||
from crochet._shutdown import (Watchdog, FunctionRegistry, _watchdog, register, | ||
_registry) | ||
from crochet._shutdown import Watchdog, FunctionRegistry, registry | ||
from ..tests import crochet_directory | ||
|
||
|
||
|
@@ -27,8 +26,8 @@ def test_shutdown(self): | |
program = """\ | ||
import threading, sys | ||
|
||
from crochet._shutdown import register, _watchdog | ||
_watchdog.start() | ||
from crochet._shutdown import registry, Watchdog, default_shutdown_condition | ||
Watchdog(default_shutdown_condition(), registry.run).start() | ||
|
||
end = False | ||
|
||
|
@@ -46,7 +45,7 @@ def stop(x, y): | |
end = True | ||
|
||
threading.Thread(target=thread).start() | ||
register(stop, 1, y=2) | ||
registry.register(stop, 1, y=2) | ||
|
||
sys.exit() | ||
""" | ||
|
@@ -69,7 +68,12 @@ class FakeThread: | |
def is_alive(self): | ||
return alive | ||
|
||
w = Watchdog(FakeThread(), lambda: done.append(True)) | ||
thread = FakeThread() | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this test could be simplified to not even have a thread, and just have a flag that gets sets to True... so correspondingly the docstring should change. |
||
def condition(): | ||
return not thread.is_alive() | ||
|
||
w = Watchdog(condition, lambda: done.append(True)) | ||
w.start() | ||
time.sleep(0.2) | ||
self.assertTrue(w.is_alive()) | ||
|
@@ -84,10 +88,7 @@ def test_api(self): | |
The module exposes a shutdown thread that will call a global | ||
registry's run(), and a register function tied to the global registry. | ||
""" | ||
self.assertIsInstance(_registry, FunctionRegistry) | ||
self.assertEqual(register, _registry.register) | ||
self.assertIsInstance(_watchdog, Watchdog) | ||
self.assertEqual(_watchdog._shutdown_function, _registry.run) | ||
self.assertIsInstance(registry, FunctionRegistry) | ||
|
||
|
||
class FunctionRegistryTests(TestCase): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In retrospect it's not clear to me why this is module-level state at all. Seems like all of the state could live inside
EventLoop
, and then there's no need to pass aFunctionRegistry
instance toEventLoop.__init__
, it would just create one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be forgetting why this was necessary though, so if you think it's too big of a change for you (or this branch) it's not necessary to do this.