actionpack#148: "address RetryPolicy ergonomic issues" (#149)

* Action reaction guaranteed to execute

* uses generic TypeVar in partialaction type hinting

* updates repr; adds typecheck for RetryPolicy max_retries

actionpack/action.py

@@ -87,7 +87,7 @@ def instruction():
         return instance
 
 
-def partialaction(name, parent: ActionType, **kwargs) -> ActionType:
+def partialaction(name, parent: T, **kwargs) -> T:
     partial__init__ = partialmethod(parent.__init__, **kwargs)
     return type(name, (parent,), {'__init__': partial__init__})
 
@@ -150,6 +150,7 @@ def _perform(
                 if should_raise:
                     raise e
                 outcome = Left(e)
+            finally:
                 if self._ActionType__reaction:
                     self._ActionType__reaction.perform(should_raise=should_raise)
 

actionpack/actions/retry_policy.py

@@ -17,6 +17,9 @@ def __init__(
         delay_between_attempts: int = 0,
         should_record: bool = False
     ):
+        if not isinstance(max_retries, int) or max_retries < 0:
+            raise self.Invalid(f'The number of max_retries must be greater than zero. Given max_retries={max_retries}.')
+
         self.action = action
         self.max_retries = max_retries
         self.delay_between_attempts = delay_between_attempts
@@ -70,10 +73,10 @@ def enact(self, with_delay: int = 0, counter: int = -1) -> Outcome:
         raise RetryPolicy.Expired(f'Max retries exceeded: {self.max_retries}.')
 
     def __repr__(self):
-        tmpl = Template('<$class_name($max_retries x $action_name$delay)>')
+        tmpl = Template('<$class_name($total_attempts x $action_name$delay)>')
         return tmpl.substitute(
             class_name=self.__class__.__name__,
-            max_retries=self.max_retries,
+            total_attempts=self.max_retries + 1,
             action_name=str(self.action),
             delay='' if not self.delay_between_attempts else f' | {self.delay_between_attempts}s delay'
         )

tests/actionpack/actions/test_retry_policy.py

@@ -5,6 +5,7 @@
 from unittest.mock import ANY
 from unittest.mock import patch
 
+from actionpack import Action
 from actionpack.action import Result
 from actionpack.actions import MakeRequest
 from actionpack.actions import RetryPolicy
@@ -132,8 +133,21 @@ def test_delay_is_bypassed_after_expiration(self, mock_session_send):
         result = action.perform(timestamp_provider=timestamp_provider)
         assert result.produced_at < timestamp_provider() + delay
 
-    def test_can_serialize(self):
-        self.assertEqual(repr(self.action), '<RetryPolicy(2 x <MakeRequest>)>')
+    def test_instantiation_fails_given_invalid_max_retries(self):
+        action = RetryPolicy[str, str](action=MakeRequest('GET', 'http://localhost'), max_retries=-1)
+        self.assertIsInstance(action, Action.Construct)
+        result = action.perform()
+        self.assertFalse(result.successful)
+        self.assertIsInstance(result.value, RetryPolicy.Invalid)
+
+    @patch('requests.Session.send')
+    def test_can_serialize(self, mock_session_send):
+        action = RetryPolicy[str, str](
+            action=MakeRequest('GET', 'http://localhost'),
+            max_retries=0,
+        )
+        self.assertEqual(repr(action), '<RetryPolicy(1 x <MakeRequest>)>')
+        self.assertEqual(repr(self.action), '<RetryPolicy(3 x <MakeRequest>)>')
 
     @patch('requests.Session.send')
     def test_can_pickle(self, mock_session_send):

tests/actionpack/test_action.py

@@ -5,6 +5,7 @@
 from oslash import Right
 from threading import Thread
 from unittest import TestCase
+from unittest.mock import patch
 
 from actionpack import Action
 from actionpack import partialaction
@@ -85,6 +86,31 @@ def fill():
         self.assertFalse(result.successful)
         self.assertIn(contents, vessel)
 
+    @patch('oslash.Left.__init__')
+    def test_can_react_to_failure_during_catastrophe(self, mock_wrapper):
+        def raise_another_failure(e):
+            raise e
+
+        mock_wrapper.side_effect = raise_another_failure
+
+        vessel = []
+        contents = 'contents'
+
+        def fill():
+            vessel.append(contents)
+
+        reaction = FakeAction(instruction_provider=fill)
+        action = FakeAction(
+            instruction_provider=self.raise_failure,
+            reaction=reaction
+        )
+
+        with self.assertRaises(Exception):
+            result = action.perform()
+            self.assertFalse(result.successful)
+
+        self.assertIn(contents, vessel)
+
     def test_Action_Construct(self):
         construct = FakeAction(typecheck='Action instantiation fails.')
         result = construct.perform()