From e355c4b9206792e09cd3f6e160f55a2c8bec6acf Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 00:04:51 -0700 Subject: [PATCH 1/7] Add test and documentation for ``recurse_effects``. --- effect/_base.py | 5 +++-- effect/test_base.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/effect/_base.py b/effect/_base.py index 6e2af14..f29ac9c 100644 --- a/effect/_base.py +++ b/effect/_base.py @@ -111,8 +111,9 @@ def perform(dispatcher, effect, recurse_effects=True): you can ignore the box by using a decorator like :func:`sync_performer` or :func:`effect.twisted.deferred_performer`. - Callbacks can return Effects, and those effects will immediately performed. - The result of the returned Effect will be passed to the next callback. + Unless ``recurse_effects`` is ``False``, Callbacks can return Effects, and + those effects will immediately performed. The result of the returned + Effect will be passed to the next callback. Note that this function does _not_ return the final result of the effect. You may instead want to use :func:`effect.sync_perform` or diff --git a/effect/test_base.py b/effect/test_base.py index 9292b80..59595bc 100644 --- a/effect/test_base.py +++ b/effect/test_base.py @@ -105,6 +105,18 @@ def test_effects_returning_effects_returning_effects(self): Effect(lambda box: calls.append("foo"))))))) self.assertEqual(calls, ['foo']) + def test_recurse_effects(self): + """ + If ``recurse_effects`` is ``False``, and an effect returns another + effect, that effect returned. + """ + calls = [] + effect = Effect(lambda box: calls.append("foo")) + perform(func_dispatcher, + Effect(lambda box: box.succeed(effect)).on(calls.append), + recurse_effects=False) + self.assertEqual(calls, [effect]) + def test_bounced(self): """ The callbacks of a performer are called after the performer returns. From e1651dc019bc42399cc4ae98de54e8e9fa00aad8 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 00:05:14 -0700 Subject: [PATCH 2/7] Add some tests from old CallbackTests. --- effect/test_base.py | 129 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 115 insertions(+), 14 deletions(-) diff --git a/effect/test_base.py b/effect/test_base.py index 59595bc..7fa750b 100644 --- a/effect/test_base.py +++ b/effect/test_base.py @@ -80,9 +80,81 @@ def test_error_with_callback(self): MatchesListwise([ MatchesException(ValueError('dispatched'))])) + def test_success_propagates_effect_exception(self): + """ + If an succes callback is specified, but a exception result occurs, + the exception is passed to the next callback. + """ + + calls = [] + intent = lambda box: box.fail( + (ValueError, ValueError('dispatched'), None)) + perform(func_dispatcher, + Effect(intent) + .on(success=lambda box: calls.append("foo")) + .on(error=calls.append)) + self.assertThat( + calls, + MatchesListwise([ + MatchesException(ValueError('dispatched'))])) + + def test_error_propagates_effect_result(self): + """ + If an error callback is specified, but a succesful result occurs, + the success is passed to the next callback. + """ + calls = [] + intent = lambda box: box.succeed("dispatched") + perform(func_dispatcher, + Effect(intent) + .on(error=lambda box: calls.append("foo")) + .on(success=calls.append)) + self.assertEqual(calls, ["dispatched"]) + + def test_callback_sucecss_exception(self): + """ + If a success callback raises an error, the exception is passed to the + error callback. + """ + calls = [] + + def raise_(_): + raise ValueError("oh dear") + + perform(func_dispatcher, + Effect(lambda box: box.succeed("foo")) + .on(success=lambda _: raise_(ValueError("oh dear"))) + .on(error=calls.append)) + self.assertThat( + calls, + MatchesListwise([ + MatchesException(ValueError('oh dear'))])) + + def test_callback_error_exception(self): + """ + If a error callback raises an error, the exception is passed to the + error callback. + """ + calls = [] + + def raise_(_): + raise ValueError("oh dear") + + intent = lambda box: box.fail( + (ValueError, ValueError('dispatched'), None)) + + perform(func_dispatcher, + Effect(intent) + .on(error=lambda _: raise_(ValueError("oh dear"))) + .on(error=calls.append)) + self.assertThat( + calls, + MatchesListwise([ + MatchesException(ValueError('oh dear'))])) + def test_effects_returning_effects(self): """ - When the effect dispatcher returns another effect, + When the effect performer returns another effect, - that effect is immediately performed with the same dispatcher, - the result of that is returned. """ @@ -105,6 +177,22 @@ def test_effects_returning_effects_returning_effects(self): Effect(lambda box: calls.append("foo"))))))) self.assertEqual(calls, ['foo']) + def test_nested_effect_exception_passes_to_outer_error_handler(self): + """ + If an inner effect raises an exception, it bubbles up and the + exc_info is passed to the outer effect's error handlers. + """ + calls = [] + intent = lambda box: box.fail( + (ValueError, ValueError('oh dear'), None)) + perform(func_dispatcher, + Effect(lambda box: box.succeed(Effect(intent))) + .on(error=calls.append)) + self.assertThat( + calls, + MatchesListwise([ + MatchesException(ValueError('oh dear'))])) + def test_recurse_effects(self): """ If ``recurse_effects`` is ``False``, and an effect returns another @@ -157,21 +245,34 @@ def get_stack(box): Effect(get_stack).on(success=lambda _: Effect(get_stack))) self.assertEqual(calls[0], calls[1]) - def test_callback_error(self): + def test_asynchronous_callback_invocation(self): """ - If a callback raises an error, the exception is passed to the error - callback. + When an Effect that is returned by a callback is resolved + *asynchronously*, the callbacks will run. + """ + results = [] + boxes = [] + dispatcher = lambda i: lambda d, i, box: boxes.append(box) + intent = object() + eff = Effect(intent).on(success=results.append) + perform(dispatcher, eff) + boxes[0].succeed('foo') + self.assertEqual(results, ['foo']) + + def test_asynchronous_callback_bounced(self): + """ + When an Effect that is returned by a callback is resolved + *asynchronously*, the callbacks are performed at the same stack depth. """ calls = [] - def raise_(_): - raise ValueError("oh dear") + def get_stack(_): + calls.append(traceback.extract_stack()) - perform(func_dispatcher, - Effect(lambda box: box.succeed("foo")) - .on(success=lambda _: raise_(ValueError("oh dear"))) - .on(error=calls.append)) - self.assertThat( - calls, - MatchesListwise([ - MatchesException(ValueError('oh dear'))])) + boxes = [] + dispatcher = lambda i: lambda d, i, box: boxes.append(box) + intent = object() + eff = Effect(intent).on(success=get_stack).on(success=get_stack) + perform(dispatcher, eff) + boxes[0].succeed('foo') + self.assertEqual(calls[0], calls[1]) From 5f7ec6d4f8878ac98c3acd99c7544bc2daec60fc Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 00:05:34 -0700 Subject: [PATCH 3/7] Get rid of unused POPOIntent. --- effect/test_base.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/effect/test_base.py b/effect/test_base.py index 7fa750b..5265e75 100644 --- a/effect/test_base.py +++ b/effect/test_base.py @@ -8,10 +8,6 @@ from ._base import Effect, NoPerformerFoundError, perform -class POPOIntent(object): - """An example effect intent.""" - - def func_dispatcher(intent): """ Simple effect dispatcher that takes callables taking a box, From 8ad6d02ddc04e47b85c6b056a7e1eee9d2f61969 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 00:08:08 -0700 Subject: [PATCH 4/7] Add docstring to test_dispatcher. --- effect/test_base.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/effect/test_base.py b/effect/test_base.py index 5265e75..f0dc45f 100644 --- a/effect/test_base.py +++ b/effect/test_base.py @@ -38,6 +38,11 @@ def test_no_performer(self): ])) def test_dispatcher(self): + """ + ``perform`` calls the provided dispatcher, with the intent of the + provided effect. The returned perform is called with the dispatcher and + intent. + """ calls = [] def dispatcher(intent): From 7421cf6da4ed859bf6f18e0d90c710cbf6649516 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 00:37:35 -0700 Subject: [PATCH 5/7] Some more twisted tests. --- effect/test_base.py | 12 +++----- effect/test_twisted.py | 65 ++++++++++++++++++++++++++++++++++-------- effect/twisted.py | 4 +-- 3 files changed, 59 insertions(+), 22 deletions(-) diff --git a/effect/test_base.py b/effect/test_base.py index f0dc45f..d85aeb3 100644 --- a/effect/test_base.py +++ b/effect/test_base.py @@ -253,10 +253,8 @@ def test_asynchronous_callback_invocation(self): """ results = [] boxes = [] - dispatcher = lambda i: lambda d, i, box: boxes.append(box) - intent = object() - eff = Effect(intent).on(success=results.append) - perform(dispatcher, eff) + eff = Effect(boxes.append).on(success=results.append) + perform(func_dispatcher, eff) boxes[0].succeed('foo') self.assertEqual(results, ['foo']) @@ -271,9 +269,7 @@ def get_stack(_): calls.append(traceback.extract_stack()) boxes = [] - dispatcher = lambda i: lambda d, i, box: boxes.append(box) - intent = object() - eff = Effect(intent).on(success=get_stack).on(success=get_stack) - perform(dispatcher, eff) + eff = Effect(boxes.append).on(success=get_stack).on(success=get_stack) + perform(func_dispatcher, eff) boxes[0].succeed('foo') self.assertEqual(calls[0], calls[1]) diff --git a/effect/test_twisted.py b/effect/test_twisted.py index 3d81e5c..6329d8f 100644 --- a/effect/test_twisted.py +++ b/effect/test_twisted.py @@ -8,14 +8,14 @@ from testtools.matchers import MatchesListwise, Equals, MatchesException from twisted.trial.unittest import SynchronousTestCase -from twisted.internet.defer import succeed, fail +from twisted.internet.defer import Deferred, succeed, fail from twisted.internet.task import Clock +from twisted.python.failure import Failure from ._base import Effect from ._intents import ( ConstantIntent, Delay, - ErrorIntent, base_dispatcher, parallel) from ._dispatcher import ComposedDispatcher @@ -26,6 +26,8 @@ make_twisted_dispatcher, perform) +from .test_base import func_dispatcher + def _dispatcher(reactor): return ComposedDispatcher([ @@ -77,8 +79,11 @@ def test_perform(self): effect.twisted.perform returns a Deferred which fires with the ultimate result of the Effect. """ - e = Effect(ConstantIntent("foo")) - d = perform(_dispatcher(None), e) + boxes = [] + e = Effect(boxes.append) + d = perform(func_dispatcher, e) + self.assertNoResult(d) + boxes[0].succeed("foo") self.assertEqual(self.successResultOf(d), 'foo') def test_perform_failure(self): @@ -86,8 +91,11 @@ def test_perform_failure(self): effect.twisted.perform fails the Deferred it returns if the ultimate result of the Effect is an exception. """ - e = Effect(ErrorIntent(ValueError('oh dear'))) - d = perform(_dispatcher(None), e) + boxes = [] + e = Effect(boxes.append) + d = perform(func_dispatcher, e) + self.assertNoResult(d) + boxes[0].fail((ValueError, ValueError("oh dear"), None)) f = self.failureResultOf(d) self.assertEqual(f.type, ValueError) self.assertEqual(str(f.value), 'oh dear') @@ -96,29 +104,62 @@ def test_perform_failure(self): class DeferredPerformerTests(TestCase): """Tests for :func:`deferred_performer`.""" - def test_deferred_performer(self): + def test_deferred_success(self): """ @deferred_performer wraps a function taking the dispatcher and intent and hooks up its Deferred result to the box. """ - deferred = succeed('foo') + deferred = Deferred() eff = Effect('meaningless').on(success=lambda x: ('success', x)) dispatcher = lambda i: deferred_performer( lambda dispatcher, intent: deferred) result = perform(dispatcher, eff) + self.assertNoResult(result) + deferred.callback("foo") self.assertEqual(self.successResultOf(result), ('success', 'foo')) - def test_deferred_performer_failure(self): + def test_deferred_failure(self): """ A failing Deferred causes error handlers to be called with an exception tuple based on the failure. """ - deferred = fail(ValueError('foo')) + deferred = Deferred() eff = Effect('meaningless').on(error=lambda e: ('error', e)) dispatcher = lambda i: deferred_performer( lambda dispatcher, intent: deferred) - result = self.successResultOf(perform(dispatcher, eff)) - self.assertThat(result, + result = perform(dispatcher, eff) + self.assertNoResult(result) + deferred.errback(Failure(ValueError('foo'))) + self.assertThat(self.successResultOf(result), + MatchesListwise([ + Equals('error'), + MatchesException(ValueError('foo'))])) + + def test_synchronous_success(self): + """ + If ``deferred_performer`` wraps a function that returns a non-deferred, + that result is the result of the effect. + """ + dispatcher = lambda i: deferred_performer( + lambda dispatcher, intent: "foo") + result = perform(dispatcher, Effect("meaningless")) + self.assertEqual( + self.successResultOf(result), + "foo") + + def test_synchronous_exception(self): + """ + If ``deferred_performer`` wraps a function that raises an exception, + the effect results in that exception. + """ + def raise_(): + raise ValueError("foo") + + dispatcher = lambda i: deferred_performer( + lambda dispatcher, intent: raise_()) + eff = Effect('meaningless').on(error=lambda e: ('error', e)) + result = perform(dispatcher, eff) + self.assertThat(self.successResultOf(result), MatchesListwise([ Equals('error'), MatchesException(ValueError('foo'))])) diff --git a/effect/twisted.py b/effect/twisted.py index 1266a87..316da20 100644 --- a/effect/twisted.py +++ b/effect/twisted.py @@ -19,7 +19,7 @@ import sys import warnings -from twisted.internet.defer import Deferred, maybeDeferred, gatherResults +from twisted.internet.defer import Deferred, gatherResults from twisted.python.failure import Failure from twisted.python.deprecate import deprecated from twisted.python.versions import Version @@ -98,7 +98,7 @@ def perform_parallel(dispatcher, parallel): :func:`twisted.internet.defer.gatherResults`. """ return gatherResults( - map(partial(maybeDeferred, perform, dispatcher), parallel.effects)) + map(partial(perform, dispatcher), parallel.effects)) def perform_delay(reactor, dispatcher, delay): From ffed04c328f00fc59db817c85550e86f5a1066fd Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 01:08:17 -0700 Subject: [PATCH 6/7] Get rid of legacy_dispatcher. rackerlabs/otter#857 removes the only extant user. --- effect/test_twisted.py | 79 +----------------------------------------- effect/twisted.py | 19 ---------- 2 files changed, 1 insertion(+), 97 deletions(-) diff --git a/effect/test_twisted.py b/effect/test_twisted.py index 6329d8f..6249340 100644 --- a/effect/test_twisted.py +++ b/effect/test_twisted.py @@ -2,13 +2,11 @@ import sys -from characteristic import attributes - from testtools import TestCase from testtools.matchers import MatchesListwise, Equals, MatchesException from twisted.trial.unittest import SynchronousTestCase -from twisted.internet.defer import Deferred, succeed, fail +from twisted.internet.defer import Deferred from twisted.internet.task import Clock from twisted.python.failure import Failure @@ -22,7 +20,6 @@ from .twisted import ( deferred_performer, exc_info_to_failure, - legacy_dispatcher, make_twisted_dispatcher, perform) @@ -197,77 +194,3 @@ def test_exc_info_to_failure(self): self.assertIs(failure.type, RuntimeError) self.assertEqual(str(failure.value), "foo") self.assertIs(failure.tb, exc_info[2]) - - -@attributes(['result']) -class _LegacyIntent(object): - def perform_effect(self, dispatcher): - return self.result - - -class _LegacyDispatchReturningIntent(object): - def perform_effect(self, dispatcher): - return dispatcher - - -class _LegacyErrorIntent(object): - def perform_effect(self, dispatcher): - raise ValueError('oh dear') - - -class LegacyDispatcherTests(TestCase): - """Tests for :func:`legacy_dispatcher`.""" - - def test_no_dispatcher(self): - """ - None is returned when there's no perform_effect method on the intent. - """ - self.assertEqual(legacy_dispatcher(None), None) - - def test_find_dispatcher(self): - """ - When there's a perform_effect method, the returned callable invokes it - with the dispatcher. - """ - intent = _LegacyDispatchReturningIntent() - d = perform(legacy_dispatcher, Effect(intent)) - self.assertEqual(self.successResultOf(d), legacy_dispatcher) - - def test_performer_deferred_result(self): - """ - When a legacy perform_effect method returns a Deferred, its result - becomes the result of the Effect. - """ - intent = _LegacyIntent(result=succeed('foo')) - d = perform(legacy_dispatcher, Effect(intent)) - self.assertEqual(self.successResultOf(d), 'foo') - - def test_performer_sync_exception(self): - """ - When a legacy perform_effect method raises an exception synchronously, - the effect fails with the exception info. - """ - intent = _LegacyErrorIntent() - eff = Effect(intent).on(error=lambda e: ('error', e)) - d = perform(legacy_dispatcher, eff) - result = self.successResultOf(d) - self.assertThat(result, - MatchesListwise([ - Equals('error'), - MatchesException(ValueError('oh dear'))])) - - def test_performer_async_exception(self): - """ - When a legacy perform_effect method returns a failing Deferred, - the effect fails with the exception info. - """ - d = fail(ValueError('oh dear!')) - intent = _LegacyIntent(result=d) - eff = Effect(intent).on(error=lambda e: ('error', e)) - d = perform(legacy_dispatcher, eff) - result = self.successResultOf(d) - self.assertThat( - result, - MatchesListwise([ - Equals('error'), - MatchesException(ValueError('oh dear!'))])) diff --git a/effect/twisted.py b/effect/twisted.py index 316da20..7efa356 100644 --- a/effect/twisted.py +++ b/effect/twisted.py @@ -49,25 +49,6 @@ def make_twisted_dispatcher(reactor): }) -@deprecated(Version('effect', 0, 1, 12), - "put performers in a TypedDispatcher and pass to perform") -def legacy_dispatcher(intent): - """ - DEPRECATED. - - A dispatcher that supports the old 'perform_effect' methods on intent - objects. We recommend specifying your performers in a - :obj:`TypeDispatcher`. - """ - method = getattr(intent, 'perform_effect', None) - if method is not None: - warnings.warn( - "Intent %r has a deprecated perform_effect method." % (intent,), - DeprecationWarning) - return deferred_performer( - lambda dispatcher, intent: method(dispatcher)) - - def deferred_performer(f): """ A decorator for performers that return Deferreds. From cd65c92edc81b0758a66154c62938e474f942fbd Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Dec 2014 01:10:50 -0700 Subject: [PATCH 7/7] Fix lint. --- effect/twisted.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/effect/twisted.py b/effect/twisted.py index 7efa356..9a55289 100644 --- a/effect/twisted.py +++ b/effect/twisted.py @@ -17,12 +17,9 @@ from functools import partial import sys -import warnings from twisted.internet.defer import Deferred, gatherResults from twisted.python.failure import Failure -from twisted.python.deprecate import deprecated -from twisted.python.versions import Version from twisted.internet.task import deferLater from ._base import perform as base_perform