Repository: canonical/operator (language: python)

Issue:
Testing the action event
Right now there is no option how to test action event with Harness.
I believed it should be added as a feature and if I'm not mistaken, it could be an easy task.

Example, how I'm testing it now:
```python
os.environ["JUJU_ACTION_NAME"] = "<action-name>"
harness.charm.framework.model._backend.action_get = mock_action_get = MagicMock()
harness.charm.framework.model._backend.action_set = mock_action_set = MagicMock()
harness.charm.framework.model._backend.action_fail = mock_action_fail = MagicMock()
harness.charm.on.<action-name>_action.emit()
```


Discussion / hints:
This should be pretty straight forward.  We'll need to add something similar to `Harness.update_config` that allows tests to set what the action params/args are when the event is triggered - e.g. `harness.run_action(*args)`.  And then implementing action-get/set/etc funcs for the testing backend.
So it is already possible to do something like:
```
harness.charm.on.some_action.emit()
```
And get the actual action event to be emitted. However `action_get`/`action_set`/etc are not implemented.
I do think a cleaner mechanism for calling an action and evaluating its results would be useful. (For the lifetime of the action that is running, we should have content for `action_get` to return the parameters that have been supplied, and we could easily return the dict represented by calls to `action_set` at the end of the function (and raise an exception? for `action_fail`).

It probably needs to be some sort of context object, because you only one those values to exist for the lifetime of the action being run.

(I ran into this again trying to test one of my old charms that has an action in it.)
I still have to implement this in scenario as well, good to keep in mind :)

For the record, this would still be very useful. Also for the record, Pietro has now included this in Scenario.

@tonyandrewmeyer is going to try to tackle this in the next few weeks.

Fix patch:
diff --git a/CHANGES.md b/CHANGES.md
index 6b420070d..2228863d3 100644
--- a/CHANGES.md
+++ b/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
diff --git a/ops/charm.py b/ops/charm.py
index 079744841..040758110 100755
--- a/ops/charm.py
+++ b/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:


Test patch:
diff --git a/ops/testing.py b/ops/testing.py
index fc007e703..86835d596 100755
--- a/ops/testing.py
+++ b/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))
diff --git a/test/test_charm.py b/test/test_charm.py
index 36f0013ae..890b99f3b 100755
--- a/test/test_charm.py
+++ b/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):
diff --git a/test/test_framework.py b/test/test_framework.py
index 704d0bb27..00a075d01 100755
--- a/test/test_framework.py
+++ b/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)
diff --git a/test/test_main.py b/test/test_main.py
index 2e17f16bb..715c20d5e 100755
--- a/test/test_main.py
+++ b/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'
diff --git a/test/test_testing.py b/test/test_testing.py
index cf68413b6..fdf3000de 100644
--- a/test/test_testing.py
+++ b/test/test_testing.py
@@ -5232,3 +5232,143 @@ def handler(args: ops.testing.ExecArgs):
         self.assertEqual(args_history[-1].group, "test_group")
         self.assertEqual(args_history[-1].group_id, 4)
         self.assertDictEqual(args_history[-1].environment, {"foo": "hello", "foobar": "barfoo"})
+
+
+class TestActions(unittest.TestCase):
+    def setUp(self):
+        action_results: typing.Dict[str, typing.Any] = {}
+        self._action_results = action_results
+
+        class ActionCharm(ops.CharmBase):
+            def __init__(self, framework: ops.Framework):
+                super().__init__(framework)
+                self.framework.observe(self.on.simple_action, self._on_simple_action)
+                self.framework.observe(self.on.fail_action, self._on_fail_action)
+                self.framework.observe(self.on.results_action, self._on_results_action)
+                self.framework.observe(
+                    self.on.log_and_results_action, self._on_log_and_results_action)
+                self.simple_was_called = False
+
+            def _on_simple_action(self, event: ops.ActionEvent):
+                """An action that doesn't generate logs, have any results, or fail."""
+                self.simple_was_called = True
+
+            def _on_fail_action(self, event: ops.ActionEvent):
+                event.fail("this will be ignored")
+                event.log("some progress")
+                event.fail("something went wrong")
+                event.log("more progress")
+                event.set_results(action_results)
+
+            def _on_log_and_results_action(self, event: ops.ActionEvent):
+                event.log("Step 1")
+                event.set_results({"result1": event.params["foo"]})
+                event.log("Step 2")
+                event.set_results({"result2": event.params.get("bar")})
+
+            def _on_results_action(self, event: ops.ActionEvent):
+                event.set_results(action_results)
+
+        self.harness = ops.testing.Harness(ActionCharm, meta='''
+            name: test
+            ''', actions='''
+            simple:
+              description: lorem ipsum
+            fail:
+              description: dolor sit amet
+            unobserved-param-tester:
+              description: consectetur adipiscing elit
+              params:
+                foo
+                bar
+              required: [foo]
+              additionalProperties: false
+            log-and-results:
+              description: sed do eiusmod tempor
+              params:
+                foo:
+                  type: string
+                  default: foo-default
+                bar:
+                  type: integer
+            results:
+              description: incididunt ut labore
+            ''')
+        self.harness.begin()
+
+    def test_before_begin(self):
+        harness = ops.testing.Harness(ops.CharmBase, meta='''
+            name: test
+            ''')
+        with self.assertRaises(RuntimeError):
+            harness.run_action("fail")
+
+    def test_invalid_action(self):
+        # This action isn't in the metadata at all.
+        with self.assertRaises(RuntimeError):
+            self.harness.run_action("another-action")
+        # Also check that we're not exposing the action with the dash to underscore replacement.
+        with self.assertRaises(RuntimeError):
+            self.harness.run_action("log_and_results")
+
+    def test_run_action(self):
+        out = self.harness.run_action("simple")
+        self.assertEqual(out.logs, [])
+        self.assertEqual(out.results, {})
+        self.assertTrue(self.harness.charm.simple_was_called)
+
+    def test_fail_action(self):
+        self._action_results.clear()
+        self._action_results["partial"] = "foo"
+        with self.assertRaises(ops.testing.ActionFailed) as cm:
+            self.harness.run_action("fail")
+        self.assertEqual(cm.exception.message, "something went wrong")
+        self.assertEqual(cm.exception.output.logs, ["some progress", "more progress"])
+        self.assertEqual(cm.exception.output.results, {"partial": "foo"})
+
+    def test_required_param(self):
+        with self.assertRaises(RuntimeError):
+            self.harness.run_action("unobserved-param-tester")
+        with self.assertRaises(RuntimeError):
+            self.harness.run_action("unobserved-param-tester", {"bar": "baz"})
+        self.harness.run_action("unobserved-param-tester", {"foo": "baz"})
+        self.harness.run_action("unobserved-param-tester", {"foo": "baz", "bar": "qux"})
+
+    def test_additional_params(self):
+        self.harness.run_action("simple", {"foo": "bar"})
+        with self.assertRaises(ops.ModelError):
+            self.harness.run_action("unobserved-param-tester", {"foo": "bar", "qux": "baz"})
+        self.harness.run_action("simple", {
+            "string": "hello",
+            "number": 28.8,
+            "object": {"a": {"b": "c"}},
+            "array": [1, 2, 3],
+            "boolean": True,
+            "null": None})
+
+    def test_logs_and_results(self):
+        out = self.harness.run_action("log-and-results")
+        self.assertEqual(out.logs, ["Step 1", "Step 2"])
+        self.assertEqual(out.results, {"result1": "foo-default", "result2": None})
+        out = self.harness.run_action("log-and-results", {"foo": "baz", "bar": 28})
+        self.assertEqual(out.results, {"result1": "baz", "result2": 28})
+
+    def test_bad_results(self):
+        # We can't have results that collide when flattened.
+        self._action_results.clear()
+        self._action_results["a"] = {"b": 1}
+        self._action_results["a.b"] = 2
+        with self.assertRaises(ValueError):
+            self.harness.run_action("results")
+        # There are some result key names we cannot use.
+        prohibited_keys = "stdout", "stdout-encoding", "stderr", "stderr-encoding"
+        for key in prohibited_keys:
+            self._action_results.clear()
+            self._action_results[key] = "foo"
+            with self.assertRaises(ops.ModelError):
+                self.harness.run_action("results")
+        # There are some additional rules around what result keys are valid.
+        self._action_results.clear()
+        self._action_results["A"] = "foo"
+        with self.assertRaises(ValueError):
+            self.harness.run_action("results")