feat: add support for testing actions with Harness (canonical#1053)

Adds support for testing actions with `ops.testing.Harness`.

### New classes in `ops.testing`

* `ActionOutput`: contains the logs and results from (simulating) running an action.
* `ActionFailed`: raised when a (simulated) action run calls `ActionEvent.fail()`.
* `_RunningAction`: bundles data about an action run.

### New `ops.testing.Harness` method: `run_action()`

* This simulates running an action with Juju, e.g. `juju run name/ord action-name param=value`. Optionally takes additional parameters, and either returns an `ActionOutput` object or raises `ActionFailed`.

### Minor changes
* Adds `additionalProperties` to `CharmMeta` (to allow validating that additional parameters are not passed)
* Removes the check in `ActionEvent`'s restore that `JUJU_ACTION_NAME` matches the event's action. This was only a sanity check, and the complexity it caused in implementing the simulated action was not worth the minimal value of the check.

### Comments on state

A reasonable amount of additional state is added to `_TestingModelBackend` (a `_RunningAction` instance, which contains `None`, and empty dict, and an `ActionOutput` instance, which itself has an empty list and empty dict), which is only required during the `run_action` call. After a `run_action` call this state still exists, but may also contain logs, results, and a failure message from the event handler.

### Other Notes

See [OP041](https://docs.google.com/document/d/1nxyiR7H7ZJZUIOfyIrEudmTg64jDJvPnPLCAeD1R7w0) (Canonical internal link).

Fixes canonical#762.

CHANGES.md

@@ -3,6 +3,7 @@
 * Added `Unit.reboot()` and `Harness.reboot_count``
 * Added `RelationMeta.optional`
 * The type of a `Handle`'s `key` was expanded from `str` to `str|None`
+* Added `Harness.run_action()`, `testing.ActionOutput`, and `testing.ActionFailed`
 * Narrowed types of `app` and `unit` in relation events to exclude `None` where applicable
 
 # 2.7.0

ops/charm.py

@@ -16,7 +16,6 @@
 
 import enum
 import logging
-import os
 import pathlib
 from typing import (
     TYPE_CHECKING,
@@ -137,12 +136,6 @@ def restore(self, snapshot: Dict[str, Any]):
 
         Not meant to be called directly by charm code.
         """
-        env_action_name = os.environ.get('JUJU_ACTION_NAME')
-        event_action_name = self.handle.kind[:-len('_action')].replace('_', '-')
-        if event_action_name != env_action_name:
-            # This could only happen if the dev manually emits the action, or from a bug.
-            raise RuntimeError('action event kind ({}) does not match current '
-                               'action ({})'.format(event_action_name, env_action_name))
         # Params are loaded at restore rather than __init__ because
         # the model is not available in __init__.
         self.params = self.framework.model._backend.action_get()
@@ -1465,6 +1458,7 @@ def __init__(self, name: str, raw: Optional[Dict[str, Any]] = None):
         self.description = raw.get('description', '')
         self.parameters = raw.get('params', {})  # {<parameter name>: <JSON Schema definition>}
         self.required = raw.get('required', [])  # [<parameter name>, ...]
+        self.additional_properties = raw.get('additionalProperties', True)
 
 
 class ContainerMeta:

ops/testing.py

@@ -135,6 +135,39 @@ class ExecResult:
 ExecHandler = Callable[[ExecArgs], Union[None, ExecResult]]
 
 
+@dataclasses.dataclass(frozen=True)
+class ActionOutput:
+    """Contains the logs and results from a :meth:`Harness.run_action` call."""
+
+    logs: List[str]
+    """Messages generated by the Charm using :meth:`ops.ActionEvent.log`."""
+
+    results: Dict[str, Any]
+    """The action's results, as set or updated by :meth:`ops.ActionEvent.set_results`."""
+
+
+class ActionFailed(Exception):  # noqa
+    """Raised when :code:`event.fail()` is called during a :meth:`Harness.run_action` call."""
+
+    message: str
+    """Optional details of the failure, as provided by :meth:`ops.ActionEvent.fail`."""
+
+    output: ActionOutput
+    """Any logs and results set by the Charm."""
+
+    def __init__(self, message: str, output: ActionOutput):
+        self.message = message
+        self.output = output
+
+
+@dataclasses.dataclass()
+class _RunningAction:
+    name: str
+    output: ActionOutput
+    parameters: Dict[str, Any]
+    failure_message: Optional[str] = None
+
+
 # noinspection PyProtectedMember
 class Harness(Generic[CharmType]):
     """This class represents a way to build up the model that will drive a test suite.
@@ -1746,6 +1779,68 @@ def reboot_count(self) -> int:
         """Number of times the charm has called :meth:`ops.Unit.reboot`."""
         return self._backend._reboot_count
 
+    def run_action(self, action_name: str,
+                   params: Optional[Dict[str, Any]] = None) -> ActionOutput:
+        """Simulates running a charm action, as with ``juju run``.
+
+        Use this only after calling :meth:`begin`.
+
+        Validates that no required parameters are missing, and that additional
+        parameters are not provided if that is not permitted. Does not validate
+        the types of the parameters - you can use the
+        `jsonschema <https://github.com/python-jsonschema/jsonschema>`_ package to
+        do this in your tests; for example::
+
+            schema = harness.charm.meta.actions["action-name"].parameters
+            try:
+                jsonschema.validate(instance=params, schema=schema)
+            except jsonschema.ValidationError:
+                # Do something about the invalid params.
+                ...
+            harness.run_action("action-name", params)
+
+        *New in version 2.8*
+
+        Args:
+            action_name: the name of the action to run, as found in ``actions.yaml``.
+            params: override the default parameter values found in ``actions.yaml``.
+                If a parameter is not in ``params``, or ``params`` is ``None``, then
+                the default value from ``actions.yaml`` will be used.
+
+        Raises:
+            ActionFailed: if :meth:`ops.ActionEvent.fail` is called. Note that this will
+              be raised at the end of the ``run_action`` call, not immediately when
+              :code:`fail()` is called, to match the run-time behaviour.
+        """
+        try:
+            action_meta = self.charm.meta.actions[action_name]
+        except KeyError:
+            raise RuntimeError(f"Charm does not have a {action_name!r} action.")
+        if params is None:
+            params = {}
+        for key in action_meta.required:
+            # Juju requires that the key is in the passed parameters, even if there is a default
+            # value in actions.yaml.
+            if key not in params:
+                raise RuntimeError(f"{key!r} parameter is required, but missing.")
+        if not action_meta.additional_properties:
+            for key in params:
+                if key not in action_meta.parameters:
+                    # Match Juju's error message.
+                    raise model.ModelError(
+                        f'additional property "{key}" is not allowed, '
+                        f'given {{"{key}":{params[key]!r}}}')
+        action_under_test = _RunningAction(action_name, ActionOutput([], {}), params)
+        handler = getattr(self.charm.on, f"{action_name.replace('-', '_')}_action")
+        self._backend._running_action = action_under_test
+        handler.emit()
+        self._backend._running_action = None
+        if action_under_test.failure_message is not None:
+            raise ActionFailed(
+                message=action_under_test.failure_message,
+                output=action_under_test.output)
+        return action_under_test.output
+
 
 def _get_app_or_unit_name(app_or_unit: AppUnitOrName) -> str:
     """Return name of given application or unit (return strings directly)."""
@@ -1963,6 +2058,7 @@ def __init__(self, unit_name: str, meta: charm.CharmMeta, config: '_RawConfig'):
         self._opened_ports: Set[model.Port] = set()
         self._networks: Dict[Tuple[Optional[str], Optional[int]], _NetworkDict] = {}
         self._reboot_count = 0
+        self._running_action: Optional[_RunningAction] = None
 
     def _validate_relation_access(self, relation_name: str, relations: List[model.Relation]):
         """Ensures that the named relation exists/has been added.
@@ -2216,17 +2312,38 @@ def _storage_remove(self, storage_id: str):
         index = int(index)
         self._storage_list[name].pop(index, None)
 
-    def action_get(self):  # type:ignore
-        raise NotImplementedError(self.action_get)  # type:ignore
-
-    def action_set(self, results):  # type:ignore
-        raise NotImplementedError(self.action_set)  # type:ignore
-
-    def action_log(self, message):  # type:ignore
-        raise NotImplementedError(self.action_log)  # type:ignore
-
-    def action_fail(self, message=''):  # type:ignore
-        raise NotImplementedError(self.action_fail)  # type:ignore
+    def action_get(self) -> Dict[str, Any]:
+        params: Dict[str, Any] = {}
+        assert self._running_action is not None
+        action_meta = self._meta.actions[self._running_action.name]
+        for name, action_meta in action_meta.parameters.items():
+            if "default" in action_meta:
+                params[name] = action_meta["default"]
+        params.update(self._running_action.parameters)
+        return params
+
+    def action_set(self, results: Dict[str, Any]):
+        assert self._running_action is not None
+        for key in ("stdout", "stderr", "stdout-encoding", "stderr-encoding"):
+            if key in results:
+                # Match Juju's error message.
+                raise model.ModelError(f'ERROR cannot set reserved action key "{key}"')
+        # Although it's not necessary, we go through the same flattening process
+        # as the real backend, in order to give Charmers advance notice if they
+        # are setting results that will not work.
+        # This also does some validation on keys to make sure that they fit the
+        # Juju constraints.
+        model._format_action_result_dict(results)  # Validate, but ignore returned value.
+        self._running_action.output.results.update(results)
+
+    def action_log(self, message: str):
+        assert self._running_action is not None
+        self._running_action.output.logs.append(message)
+
+    def action_fail(self, message: str = ''):
+        assert self._running_action is not None
+        # If fail is called multiple times, Juju only retains the most recent failure message.
+        self._running_action.failure_message = message
 
     def network_get(self, endpoint_name: str, relation_id: Optional[int] = None) -> '_NetworkDict':
         data = self._networks.get((endpoint_name, relation_id))

test/test_charm.py

@@ -431,10 +431,10 @@ def _get_action_test_meta(cls):
   title: foo-bar
 start:
   description: "Start the unit."
+  additionalProperties: false
 ''')
 
     def _setup_test_action(self):
-        os.environ['JUJU_ACTION_NAME'] = 'foo-bar'
         fake_script(self, 'action-get', """echo '{"foo-name": "name", "silent": true}'""")
         fake_script(self, 'action-set', "")
         fake_script(self, 'action-log', "")
@@ -476,11 +476,6 @@ def _on_start_action(self, event: ops.ActionEvent):
             ['action-fail', "test-fail"],
         ])
 
-        # Make sure that action events that do not match the current context are
-        # not possible to emit by hand.
-        with self.assertRaises(RuntimeError):
-            charm.on.start_action.emit()
-
     def test_invalid_action_results(self):
 
         class MyCharm(ops.CharmBase):

test/test_framework.py

@@ -1799,8 +1799,7 @@ class CustomEvents(ops.ObjectEvents):
 
         with patch('sys.stderr', new_callable=io.StringIO):
             with patch('pdb.runcall') as mock:
-                with patch.dict(os.environ, {'JUJU_ACTION_NAME': 'foobar'}):
-                    publisher.foobar_action.emit()
+                publisher.foobar_action.emit()
 
         self.assertEqual(mock.call_count, 1)
         self.assertFalse(observer.called)
@@ -1820,8 +1819,7 @@ class CustomEvents(ops.ObjectEvents):
 
         with patch('sys.stderr', new_callable=io.StringIO):
             with patch('pdb.runcall') as mock:
-                with patch.dict(os.environ, {'JUJU_ACTION_NAME': 'foobar'}):
-                    publisher.foobar_action.emit()
+                publisher.foobar_action.emit()
 
         self.assertEqual(mock.call_count, 1)
         self.assertFalse(observer.called)

test/test_main.py

@@ -436,7 +436,7 @@ def _simulate_event(self, event_spec: EventSpec):
             if event_spec.env_var == 'JUJU_ACTION_NAME':
                 event_dir = 'actions'
             else:
-                raise RuntimeError('invalid envar name specified for a action event')
+                raise RuntimeError('invalid envar name specified for an action event')
         else:
             event_filename = event_spec.event_name.replace('_', '-')
             event_dir = 'hooks'