Fix bad cherry-picking/merging.

testing/src/scenario/_consistency_checker.py

@@ -170,9 +170,15 @@ def check_event_consistency(
     errors: List[str] = []
     warnings: List[str] = []
 
-    # custom event: can't make assumptions about its name and its semantics
-    # todo: should we then just skip the other checks?
     if not event._is_builtin_event(charm_spec):
+        # This is a custom event - we can't make assumptions about its name and
+        # semantics. It doesn't really make sense to do checks that are designed
+        # for relations, workloads, and so on - most likely those will end up
+        # with false positives. Realistically, we can't know about what the
+        # requirements for the custom event are (in terms of the state), so we
+        # skip everything here. Perhaps in the future, custom events could
+        # optionally include some sort of state metadata that made testing
+        # consistency possible?
         warnings.append(
             "this is a custom event; if its name makes it look like a builtin one "
             "(e.g. a relation event, or a workload event), you might get some false-negative "

testing/src/scenario/_runtime.py

@@ -84,13 +84,11 @@ def _get_event_env(self, state: "State", event: "_Event", charm_root: Path):
             "JUJU_MODEL_NAME": state.model.name,
             "JUJU_MODEL_UUID": state.model.uuid,
             "JUJU_CHARM_DIR": str(charm_root.absolute()),
-            # todo consider setting pwd, (python)path
         }
 
         if event._is_action_event and (action := event.action):
             env.update(
                 {
-                    # TODO: we should check we're doing the right thing here.
                     "JUJU_ACTION_NAME": action.name.replace("_", "-"),
                     "JUJU_ACTION_UUID": action.id,
                 },
@@ -136,7 +134,7 @@ def _get_event_env(self, state: "State", event: "_Event", charm_root: Path):
                 else:
                     logger.warning(
                         "remote unit ID unset; no remote unit data present. "
-                        "Is this a realistic scenario?",  # TODO: is it?
+                        "Is this a realistic scenario?",
                     )
 
             if remote_unit_id is not None:
@@ -188,14 +186,15 @@ def _get_event_env(self, state: "State", event: "_Event", charm_root: Path):
     @staticmethod
     def _wrap(charm_type: Type["CharmType"]) -> Type["CharmType"]:
         # dark sorcery to work around framework using class attrs to hold on to event sources
-        # todo this should only be needed if we call play multiple times on the same runtime.
-        #  can we avoid it?
+        # this should only be needed if we call play multiple times on the same runtime.
         class WrappedEvents(charm_type.on.__class__):
+            """The charm's event sources, but wrapped."""
             pass
 
         WrappedEvents.__name__ = charm_type.on.__class__.__name__
 
         class WrappedCharm(charm_type):
+            """The test charm's type, but with events wrapped."""
             on = WrappedEvents()
 
         WrappedCharm.__name__ = charm_type.__name__
@@ -276,7 +275,6 @@ def _virtual_charm_root(self):
     def _exec_ctx(self, ctx: "Context"):
         """python 3.8 compatibility shim"""
         with self._virtual_charm_root() as temporary_charm_root:
-            # TODO: allow customising capture_events
             with capture_events(
                 include_deferred=ctx.capture_deferred_events,
                 include_framework=ctx.capture_framework_events,
@@ -298,9 +296,6 @@ def exec(
         This will set the environment up and call ops.main().
         After that it's up to ops.
         """
-        # todo consider forking out a real subprocess and do the mocking by
-        #  mocking hook tool executables
-
         from ._consistency_checker import check_consistency  # avoid cycles
 
         check_consistency(state, event, self._charm_spec, self._juju_version)

testing/src/scenario/context.py

@@ -2,6 +2,12 @@
 # Copyright 2023 Canonical Ltd.
 # See LICENSE file for licensing details.
 
+"""Test Context
+
+The test `Context` object provides the context of the wider Juju system that the
+specific `State` exists in, and the events that can be executed on that `State`.
+"""
+
 from __future__ import annotations
 
 import functools