From 1e8ff8053a6630a88fddc05d1982ff07be8f6db2 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 24 Apr 2024 14:31:41 +1200 Subject: [PATCH 01/23] Support 'ctx.on.event_name' for specifying events. --- scenario/context.py | 288 ++++++++++++++++++++++++++++-- scenario/runtime.py | 14 +- scenario/state.py | 4 + tests/test_context_on.py | 291 +++++++++++++++++++++++++++++++ tests/test_e2e/test_relations.py | 19 +- tox.ini | 2 +- 6 files changed, 599 insertions(+), 19 deletions(-) create mode 100644 tests/test_context_on.py diff --git a/scenario/context.py b/scenario/context.py index 9de78b1c..9f356496 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -5,19 +5,38 @@ import tempfile from contextlib import contextmanager from pathlib import Path -from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Type, Union, cast +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + List, + Optional, + Self, + Type, + Union, + cast, +) from ops import CharmBase, EventBase from scenario.logger import logger as scenario_logger from scenario.runtime import Runtime -from scenario.state import Action, Event, MetadataNotFoundError, _CharmSpec +from scenario.state import ( + Action, + Container, + Event, + MetadataNotFoundError, + Secret, + Storage, + _CharmSpec, +) if TYPE_CHECKING: # pragma: no cover from ops.testing import CharmType from scenario.ops_main_mock import Ops - from scenario.state import JujuLogLine, State, _EntityStatus + from scenario.state import AnyRelation, JujuLogLine, State, _EntityStatus PathLike = Union[str, Path] @@ -154,6 +173,164 @@ def _get_output(self): return self._ctx._finalize_action(self._ctx.output_state) # noqa +@dataclasses.dataclass +class _EventSource: + kind: str + + +@dataclasses.dataclass +class _SecretEventSource(_EventSource): + _secret: Optional[Secret] = None + _revision: Optional[int] = None + + def __call__(self, secret: Secret, revision: Optional[int] = None) -> Self: + """Link to a specific scenario.Secret object.""" + self._secret = secret + self._revision = revision + return self + + +@dataclasses.dataclass +class _RelationEventSource(_EventSource): + name: str + _unit_id: Optional[str] = None + _departing_unit_id: Optional[str] = None + + def __call__( + self, + unit: Optional[str] = None, + departing_unit: Optional[str] = None, + ) -> Self: + self._unit_id = unit + self._departing_unit_id = departing_unit + return self + + +@dataclasses.dataclass +class _StorageEventSource(_EventSource): + name: str + + +@dataclasses.dataclass +class _ContainerEventSource(_EventSource): + name: str + + +@dataclasses.dataclass +class _ActionEventSource(_EventSource): + name: str + _action: Optional[Action] = None + + def __call__(self, action: Action) -> Self: + """Provide a scenario.Action object, in order to specify params or id.""" + if action.name != self.name: + raise RuntimeError("WRITE AN ERROR MESSAGE HERE") + self._action = action + return self + + +class _CharmEvents: + """Events generated by Juju pertaining to application lifecycle. + + By default, the events listed as attributes of this class will be + provided via the :attr:`Context.on` attribute. For example:: + + ctx.run(ctx.on.config_changed, state) + + In addition to the events listed as attributes of this class, + dynamically-named events will also be defined based on the charm's + metadata for relations, storage, actions, and containers. These named + events may be accessed as ``ctx.on._``, for example:: + + ctx.run(ctx.on.workload_pebble_ready, state) + + See also the :class:`ops.CharmEvents` class. + """ + + # TODO: There are lots of suffix definitions in state - we should be able to re-use those. + install = _EventSource("install") + start = _EventSource("start") + stop = _EventSource("stop") + remove = _EventSource("remove") + update_status = _EventSource("update_status") + config_changed = _EventSource("config_changed") + upgrade_charm = _EventSource("upgrade_charm") + pre_series_upgrade = _EventSource("pre_series_upgrade") + post_series_upgrade = _EventSource("post_series_upgrade") + leader_elected = _EventSource("leader_elected") + secret_changed = _SecretEventSource("secret_changed") + secret_expired = _SecretEventSource("secret_expired") + secret_rotate = _SecretEventSource("secret_rotate") + secret_remove = _SecretEventSource("secret_remove") + collect_app_status = _EventSource("collect_app_status") + collect_unit_status = _EventSource("collect_unit_status") + + def __init__(self, charm_spec: _CharmSpec): + for relation_name, _ in charm_spec.get_all_relations(): + relation_name = relation_name.replace("-", "_") + setattr( + self, + f"{relation_name}_relation_created", + _RelationEventSource("relation_created", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_joined", + _RelationEventSource("relation_joined", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_changed", + _RelationEventSource("relation_changed", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_departed", + _RelationEventSource("relation_departed", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_broken", + _RelationEventSource("relation_broken", relation_name), + ) + + for storage_name in charm_spec.meta.get("storage", ()): + storage_name = storage_name.replace("-", "_") + setattr( + self, + f"{storage_name}_storage_attached", + _StorageEventSource("storage_attached", storage_name), + ) + setattr( + self, + f"{storage_name}_storage_detaching", + _StorageEventSource("storage_detaching", storage_name), + ) + + for action_name in charm_spec.actions or (): + action_name = action_name.replace("-", "_") + setattr( + self, + f"{action_name}_action", + _ActionEventSource("action", action_name), + ) + + for container_name in charm_spec.meta.get("containers", ()): + container_name = container_name.replace("-", "_") + setattr( + self, + f"{container_name}_pebble_ready", + _ContainerEventSource("pebble_ready", container_name), + ) + + +# setattr( +# self, +# f"{container_name}_pebble_custom_notice", +# _ContainerEventSource("pebble_custom_notice", container_name), +# ) + + class Context: """Scenario test execution context.""" @@ -291,6 +468,8 @@ def __init__( self._action_results: Optional[Dict[str, str]] = None self._action_failure: Optional[str] = None + self.on = _CharmEvents(self.charm_spec) + def _set_output_state(self, output_state: "State"): """Hook for Runtime to set the output state.""" self._output_state = output_state @@ -353,23 +532,104 @@ def _record_status(self, state: "State", is_app: bool): self.unit_status_history.append(cast("_EntityStatus", state.unit_status)) @staticmethod - def _coalesce_action(action: Union[str, Action]) -> Action: + def _coalesce_action(action: Union[str, Action, _ActionEventSource]) -> Action: """Validate the action argument and cast to Action.""" if isinstance(action, str): return Action(action) + if isinstance(action, _ActionEventSource): + if action._action: + return action._action + return Action(action.name) + if not isinstance(action, Action): raise InvalidActionError( f"Expected Action or action name; got {type(action)}", ) return action + # TODO: These don't really need to be List, probably Iterable is fine? Really ought to be Mapping ... @staticmethod - def _coalesce_event(event: Union[str, Event]) -> Event: + def _coalesce_event( + event: Union[str, Event, _EventSource], + *, + containers: Optional[List[Container]] = None, + storages: Optional[List[Storage]] = None, + relations: Optional[List["AnyRelation"]] = None, + ) -> Event: """Validate the event argument and cast to Event.""" if isinstance(event, str): event = Event(event) + if isinstance(event, _EventSource): + if event.kind == "action": + raise InvalidEventError( + "Cannot Context.run() action events. " + "Use Context.run_action instead.", + ) + if isinstance(event, _SecretEventSource): + if (secret := event._secret) is None: + raise InvalidEventError( + "A secret must be provided, for example: " + "ctx.run(ctx.on.secret_changed(secret=secret), state)", + ) + else: + secret = None + secret_revision = getattr(event, "_revision", None) + # TODO: These can probably do Event.bind() + if isinstance(event, _StorageEventSource): + # TODO: It would be great if this was a mapping, not a list. + for storage in storages or (): + if storage.name == event.name: + break + else: + raise InvalidEventError( + f"Attempting to run {event.name}_{event.kind}, but " + f"{event.name} is not a storage in the state.", + ) + else: + storage = None + if isinstance(event, _ContainerEventSource): + # TODO: It would be great if this was a mapping, not a list. + for container in containers or (): + if container.name == event.name: + break + else: + raise InvalidEventError( + f"Attempting to run {event.name}_{event.kind}, but " + f"{event.name} is not a container in the state.", + ) + else: + container = None + if isinstance(event, _RelationEventSource): + # TODO: It would be great if this was a mapping, not a list. + for relation in relations or (): + if relation.endpoint == event.name: + break + else: + raise InvalidEventError( + f"Attempting to run {event.name}_{event.kind}, but " + f"{event.name} is not a relation in the state.", + ) + else: + relation = None + relation_remote_unit_id = getattr(event, "_unit_id", None) + relation_departed_unit_id = getattr(event, "_departing_unit_id", None) + if hasattr(event, "name"): + path = f"{event.name}_{event.kind}" # type: ignore + else: + path = event.kind + event = Event( + path, + secret=secret, + secret_revision=secret_revision, + storage=storage, + container=container, + relation=relation, + relation_remote_unit_id=relation_remote_unit_id, + relation_departed_unit_id=relation_departed_unit_id, + ) + if not isinstance(event, Event): raise InvalidEventError(f"Expected Event | str, got {type(event)}") @@ -435,16 +695,21 @@ def action_manager( @contextmanager def _run_event( self, - event: Union["Event", str], + event: Union["_EventSource", "Event", str], state: "State", ): - _event = self._coalesce_event(event) + _event = self._coalesce_event( + event, + containers=state.containers, + storages=state.storage, + relations=state.relations, + ) with self._run(event=_event, state=state) as ops: yield ops def run( self, - event: Union["Event", str], + event: Union["_EventSource", "Event", str], state: "State", pre_event: Optional[Callable[[CharmBase], None]] = None, post_event: Optional[Callable[[CharmBase], None]] = None, @@ -454,7 +719,8 @@ def run( Calling this function will call ``ops.main`` and set up the context according to the specified ``State``, then emit the event on the charm. - :arg event: the Event that the charm will respond to. Can be a string or an Event instance. + :arg event: the Event that the charm will respond to. Can be a string or an Event instance + or an _EventSource. :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Event. :arg pre_event: callback to be invoked right before emitting the event on the newly @@ -479,7 +745,7 @@ def run( def run_action( self, - action: Union["Action", str], + action: Union["_ActionEventSource", "Action", str], state: "State", pre_event: Optional[Callable[[CharmBase], None]] = None, post_event: Optional[Callable[[CharmBase], None]] = None, @@ -531,7 +797,7 @@ def _finalize_action(self, state_out: "State"): @contextmanager def _run_action( self, - action: Union["Action", str], + action: Union["_ActionEventSource", "Action", str], state: "State", ): _action = self._coalesce_action(action) diff --git a/scenario/runtime.py b/scenario/runtime.py index a6aebab0..eb05b211 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -216,7 +216,9 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): remote_unit_id = event.relation_remote_unit_id # don't check truthiness because remote_unit_id could be 0 - if remote_unit_id is None: + if remote_unit_id is None and not event.name.endswith( + ("_relation_created", "relation_broken"), + ): remote_unit_ids = relation._remote_unit_ids # pyright: ignore if len(remote_unit_ids) == 1: @@ -243,7 +245,12 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): remote_unit = f"{remote_app_name}/{remote_unit_id}" env["JUJU_REMOTE_UNIT"] = remote_unit if event.name.endswith("_relation_departed"): - env["JUJU_DEPARTING_UNIT"] = remote_unit + if event.relation_departed_unit_id: + env[ + "JUJU_DEPARTING_UNIT" + ] = f"{remote_app_name}/{event.relation_departed_unit_id}" + else: + env["JUJU_DEPARTING_UNIT"] = remote_unit if container := event.container: env.update({"JUJU_WORKLOAD_NAME": container.name}) @@ -258,6 +265,9 @@ def _get_event_env(self, state: "State", event: "Event", charm_root: Path): "JUJU_SECRET_LABEL": secret.label or "", }, ) + # Don't check truthiness because revision could be 0. + if event.secret_revision is not None: + env["JUJU_SECRET_REVISION"] = str(event.secret_revision) return env diff --git a/scenario/state.py b/scenario/state.py index 8d14e493..789ae064 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -1210,9 +1210,13 @@ class Event(_DCBase): relation: Optional["AnyRelation"] = None # and the name of the remote unit this relation event is about relation_remote_unit_id: Optional[int] = None + # and the name of the unit that is departing if this is -relation-departed. + relation_departed_unit_id: Optional[int] = None # if this is a secret event, the secret it refers to secret: Optional[Secret] = None + # if this is a secret-removed or secret-expired event, the secret revision it refers to + secret_revision: Optional[int] = None # if this is a workload (container) event, the container it refers to container: Optional[Container] = None diff --git a/tests/test_context_on.py b/tests/test_context_on.py new file mode 100644 index 00000000..88db6d6a --- /dev/null +++ b/tests/test_context_on.py @@ -0,0 +1,291 @@ +import ops +import pytest + +import scenario + +META = { + "name": "context-charm", + "containers": { + "bar": {}, + }, + "requires": { + "baz": { + "interface": "charmlink", + } + }, + "storage": { + "foo": { + "type": "filesystem", + } + }, +} +ACTIONS = { + "act": { + "params": { + "param": { + "description": "some parameter", + "type": "string", + "default": "", + } + } + }, +} + + +class ContextCharm(ops.CharmBase): + def __init__(self, framework): + super().__init__(framework) + self.observed = [] + for event in self.on.events().values(): + framework.observe(event, self._on_event) + + def _on_event(self, event): + self.observed.append(event) + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("install", ops.InstallEvent), + ("start", ops.StartEvent), + ("stop", ops.StopEvent), + ("remove", ops.RemoveEvent), + ("update_status", ops.UpdateStatusEvent), + ("config_changed", ops.ConfigChangedEvent), + ("upgrade_charm", ops.UpgradeCharmEvent), + ("pre_series_upgrade", ops.PreSeriesUpgradeEvent), + ("post_series_upgrade", ops.PostSeriesUpgradeEvent), + ("leader_elected", ops.LeaderElectedEvent), + ], +) +def test_simple_events(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + # These look like: + # ctx.run(ctx.on.install, state) + with ctx.manager(getattr(ctx.on, event_name), scenario.State()) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + assert isinstance(mgr.charm.observed[0], event_kind) + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("secret_changed", ops.SecretChangedEvent), + ("secret_rotate", ops.SecretRotateEvent), + ], +) +def test_simple_secret_events(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + secret = scenario.Secret("secret:123", {0: {"password": "xxxx"}}, owner=None) + state_in = scenario.State(secrets=[secret]) + # These look like: + # ctx.run(ctx.on.secret_changed(secret=secret), state) + # The secret must always be passed because the same event name is used for + # all secrets. + with ctx.manager(getattr(ctx.on, event_name)(secret=secret), state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, event_kind) + assert event.secret.id == secret.id + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("secret_expired", ops.SecretExpiredEvent), + ("secret_remove", ops.SecretRemoveEvent), + ], +) +def test_revision_secret_events(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + secret = scenario.Secret( + "secret:123", {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, owner=None + ) + state_in = scenario.State(secrets=[secret]) + # These look like: + # ctx.run(ctx.on.secret_expired(secret=secret, revision=revision), state) + # The secret and revision must always be passed because the same event name + # is used for all secrets. + with ctx.manager( + getattr(ctx.on, event_name)(secret=secret, revision=42), state_in + ) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, event_kind) + assert event.secret.id == secret.id + assert event.revision == 42 + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("foo_storage_attached", ops.StorageAttachedEvent), + ("foo_storage_detaching", ops.StorageDetachingEvent), + ], +) +def test_storage_events(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) + # These look like: + # ctx.run(ctx.on.foo_storage_attached, state) + # The storage is inferred from the event name. + with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, event_kind) + assert event.storage.name == storage.name + assert event.storage.index == storage.index + + +def test_action_event_no_params(): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + # These look like: + # ctx.run_action(ctx.on.act_action, state) + # The action is inferred from the event name. + with ctx.action_manager(ctx.on.act_action, scenario.State()) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, ops.ActionEvent) + + +def test_action_event_with_params(): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + action = scenario.Action("act", {"param": "hello"}) + # These look like: + # ctx.run_action(ctx.on.act_action(action=action), state) + # So that any parameters can be included and the ID can be customised. + with ctx.action_manager(ctx.on.act_action(action=action), scenario.State()) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, ops.ActionEvent) + assert event.id == action.id + assert event.params["param"] == action.params["param"] + + +def test_pebble_ready_event(): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + container = scenario.Container("bar", can_connect=True) + state_in = scenario.State(containers=[container]) + # These look like: + # ctx.run(ctx.on.bar_pebble_ready, state) + # The container/workload is inferred from the event name. + with ctx.manager(ctx.on.bar_pebble_ready, state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, ops.PebbleReadyEvent) + assert event.workload.name == container.name + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("baz_relation_created", ops.RelationCreatedEvent), + ("baz_relation_broken", ops.RelationBrokenEvent), + ], +) +def test_relation_app_events(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + relation = scenario.Relation("baz") + state_in = scenario.State(relations=[relation]) + # These look like: + # ctx.run(ctx.on.baz_relation_created, state) + # The relation is inferred from the event name. + with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, event_kind) + assert event.relation.id == relation.relation_id + assert event.app.name == relation.remote_app_name + assert event.unit is None + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("baz_relation_joined", ops.RelationJoinedEvent), + ("baz_relation_changed", ops.RelationChangedEvent), + ], +) +def test_relation_unit_events_default_unit(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + relation = scenario.Relation("baz", remote_units_data={1: {"x": "y"}}) + state_in = scenario.State(relations=[relation]) + # These look like: + # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) + # The relation is inferred from the event name and the unit is chosen + # automatically. + with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, event_kind) + assert event.relation.id == relation.relation_id + assert event.app.name == relation.remote_app_name + assert event.unit.name == "remote/1" + + +@pytest.mark.parametrize( + "event_name, event_kind", + [ + ("baz_relation_joined", ops.RelationJoinedEvent), + ("baz_relation_changed", ops.RelationChangedEvent), + ], +) +def test_relation_unit_events(event_name, event_kind): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + relation = scenario.Relation( + "baz", remote_units_data={1: {"x": "y"}, 2: {"x": "z"}} + ) + state_in = scenario.State(relations=[relation]) + # These look like: + # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) + # The relation is inferred from the event name, and an explicit unit choice is provided. + with ctx.manager(getattr(ctx.on, event_name)(unit=2), state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, event_kind) + assert event.relation.id == relation.relation_id + assert event.app.name == relation.remote_app_name + assert event.unit.name == "remote/2" + + +def test_relation_departed_event(): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + relation = scenario.Relation("baz") + state_in = scenario.State(relations=[relation]) + # These look like: + # ctx.run(ctx.on.baz_relation_departed(unit=unit_ordinal, departing_unit=unit_ordinal), state) + # The relation is inferred from the event name, and an explicit unit choice is provided for + # both the triggering unit and the departing unit. + with ctx.manager( + ctx.on.baz_relation_departed(unit=2, departing_unit=1), state_in + ) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) + event = mgr.charm.observed[0] + assert isinstance(event, ops.RelationDepartedEvent) + assert event.relation.id == relation.relation_id + assert event.app.name == relation.remote_app_name + assert event.unit.name == "remote/2" + assert event.departing_unit.name == "remote/1" diff --git a/tests/test_e2e/test_relations.py b/tests/test_e2e/test_relations.py index 212e12a6..a3517368 100644 --- a/tests/test_e2e/test_relations.py +++ b/tests/test_e2e/test_relations.py @@ -5,6 +5,8 @@ CharmBase, CharmEvents, CollectStatusEvent, + RelationBrokenEvent, + RelationCreatedEvent, RelationDepartedEvent, RelationEvent, ) @@ -218,7 +220,11 @@ def callback(charm: CharmBase, event): return assert event.app # that's always present - assert event.unit + # .unit is always None for created and broken. + if isinstance(event, (RelationCreatedEvent, RelationBrokenEvent)): + assert event.unit is None + else: + assert event.unit assert (evt_name == "departed") is bool(getattr(event, "departing_unit", False)) mycharm._call = callback @@ -239,9 +245,11 @@ def callback(charm: CharmBase, event): }, ) - assert ( - "remote unit ID unset, and multiple remote unit IDs are present" in caplog.text - ) + if evt_name not in ("created", "broken"): + assert ( + "remote unit ID unset, and multiple remote unit IDs are present" + in caplog.text + ) def test_relation_default_unit_data_regular(): @@ -297,7 +305,8 @@ def callback(charm: CharmBase, event): }, ) - assert "remote unit ID unset; no remote unit data present" in caplog.text + if evt_name not in ("created", "broken"): + assert "remote unit ID unset; no remote unit data present" in caplog.text @pytest.mark.parametrize("data", (set(), {}, [], (), 1, 1.0, None, b"")) diff --git a/tox.ini b/tox.ini index b670ca95..b6f24de8 100644 --- a/tox.ini +++ b/tox.ini @@ -57,7 +57,7 @@ deps = coverage[toml] isort commands = - black --check {[vars]tst_path} {[vars]src_path} + black --check {[vars]tst_path} isort --check-only --profile black {[vars]tst_path} [testenv:fmt] From b00cf6879d789cd393e7bbb130aff5209217dce3 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 24 Apr 2024 14:34:16 +1200 Subject: [PATCH 02/23] Fix typo in comment. --- tests/test_context_on.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 88db6d6a..eaf89422 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -228,7 +228,7 @@ def test_relation_unit_events_default_unit(event_name, event_kind): relation = scenario.Relation("baz", remote_units_data={1: {"x": "y"}}) state_in = scenario.State(relations=[relation]) # These look like: - # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) + # ctx.run(ctx.on.baz_relation_changed, state) # The relation is inferred from the event name and the unit is chosen # automatically. with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: From 9af61a6b97752c536d54362a6122f88ca6fb2905 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 24 Apr 2024 16:02:41 +1200 Subject: [PATCH 03/23] Remove support for directly running custom events. --- README.md | 48 +----------------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/README.md b/README.md index f34abfe2..6cb9e36e 100644 --- a/README.md +++ b/README.md @@ -1117,58 +1117,12 @@ foo_relation = scenario.Relation('foo') foo_relation.changed_event.deferred(handler=MyCharm._on_foo_relation_changed) ``` -## Fine-tuning - -The deferred helper Scenario provides will not support out of the box all custom event subclasses, or events emitted by -charm libraries or objects other than the main charm class. - -For general-purpose usage, you will need to instantiate DeferredEvent directly. - -```python -import scenario - -my_deferred_event = scenario.DeferredEvent( - handle_path='MyCharm/MyCharmLib/on/database_ready[1]', - owner='MyCharmLib', # the object observing the event. Could also be MyCharm. - observer='_on_database_ready' -) -``` - # Emitting custom events -While the main use case of Scenario is to emit Juju events, i.e. the built-in `start`, `install`, `*-relation-changed`, -etc..., it can be sometimes handy to directly trigger custom events defined on arbitrary Objects in your hierarchy. - Suppose your charm uses a charm library providing an `ingress_provided` event. -The 'proper' way to emit it is to run the event that causes that custom event to be emitted by the library, whatever +The proper way to emit it is to run the event that causes that custom event to be emitted by the library, whatever that may be, for example a `foo-relation-changed`. -However, that may mean that you have to set up all sorts of State and mocks so that the right preconditions are met and -the event is emitted at all. - -If for whatever reason you don't want to do that and you attempt to run that event directly you will get an error: - -```python -import scenario - -scenario.Context(...).run("ingress_provided", scenario.State()) # raises scenario.ops_main_mock.NoObserverError -``` - -This happens because the framework, by default, searches for an event source named `ingress_provided` in `charm.on`, but -since the event is defined on another Object, it will fail to find it. - -You can prefix the event name with the path leading to its owner to tell Scenario where to find the event source: - -```python -import scenario - -scenario.Context(...).run("my_charm_lib.on.foo", scenario.State()) -``` - -This will instruct Scenario to emit `my_charm.my_charm_lib.on.foo`. - -(always omit the 'root', i.e. the charm framework key, from the path) - # Live charm introspection Scenario is a black-box, state-transition testing framework. It makes it trivial to assert that a status went from A to From 136a823d26c7c06de35f521ee74f123c4f417727 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 4 Jun 2024 16:44:06 +1200 Subject: [PATCH 04/23] Update tests and docs to match final (hopefully\!) API decision. --- README.md | 46 +++++------ tests/test_charm_spec_autoload.py | 4 +- tests/test_context_on.py | 130 +++++++++++++++++++++--------- tests/test_e2e/test_deferred.py | 8 +- tests/test_e2e/test_event.py | 4 +- tests/test_e2e/test_juju_log.py | 2 +- tests/test_e2e/test_ports.py | 4 +- tests/test_e2e/test_secrets.py | 4 +- tests/test_e2e/test_status.py | 10 +-- tests/test_e2e/test_storage.py | 4 +- tests/test_plugin.py | 2 +- 11 files changed, 134 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index 016cf4d4..ba2f10c3 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ class MyCharm(ops.CharmBase): def test_scenario_base(): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) - out = ctx.run(scenario.Event("start"), scenario.State()) + out = ctx.run(ctx.on.start(), scenario.State()) assert out.unit_status == ops.UnknownStatus() ``` @@ -119,7 +119,7 @@ class MyCharm(ops.CharmBase): @pytest.mark.parametrize('leader', (True, False)) def test_status_leader(leader): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) - out = ctx.run('start', scenario.State(leader=leader)) + out = ctx.run(ctx.on.start(), scenario.State(leader=leader)) assert out.unit_status == ops.ActiveStatus('I rule' if leader else 'I am ruled') ``` @@ -181,7 +181,7 @@ from charm import MyCharm def test_statuses(): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) - out = ctx.run('start', scenario.State(leader=False)) + out = ctx.run(ctx.on.start(), scenario.State(leader=False)) assert ctx.unit_status_history == [ ops.UnknownStatus(), ops.MaintenanceStatus('determining who the ruler is...'), @@ -210,7 +210,7 @@ import ops import scenario # ... -ctx.run('start', scenario.State(unit_status=ops.ActiveStatus('foo'))) +ctx.run(ctx.start(), scenario.State(unit_status=ops.ActiveStatus('foo'))) assert ctx.unit_status_history == [ ops.ActiveStatus('foo'), # now the first status is active: 'foo'! # ... @@ -247,7 +247,7 @@ import scenario def test_foo(): ctx = scenario.Context(...) - ctx.run('start', ...) + ctx.run(ctx.on.start(), ...) assert len(ctx.emitted_events) == 1 assert isinstance(ctx.emitted_events[0], ops.StartEvent) @@ -267,7 +267,7 @@ def test_emitted_full(): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run("start", scenario.State(deferred=[scenario.Event("update-status").deferred(MyCharm._foo)])) + ctx.run(ctx.on.start(), scenario.State(deferred=[scenario.Event("update-status").deferred(MyCharm._foo)])) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ @@ -294,7 +294,7 @@ import scenario with capture_events() as emitted: ctx = scenario.Context(...) state_out = ctx.run( - "update-status", + ctx.on.update_status(), scenario.State(deferred=[scenario.DeferredEvent("start", ...)]) ) @@ -357,7 +357,7 @@ def test_relation_data(): ]) ctx = scenario.Context(MyCharm, meta={"name": "foo"}) - state_out = ctx.run('start', state_in) + state_out = ctx.run(ctx.on.start(), state_in) assert state_out.relations[0].local_unit_data == {"abc": "baz!"} # you can do this to check that there are no other differences: @@ -415,7 +415,7 @@ state_in = scenario.State(relations=[ peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}}, )]) -scenario.Context(..., unit_id=1).run("start", state_in) # invalid: this unit's id cannot be the ID of a peer. +scenario.Context(..., unit_id=1).run(ctx.on.start(), state_in) # invalid: this unit's id cannot be the ID of a peer. ``` @@ -623,7 +623,7 @@ def test_pebble_push(): meta={"name": "foo", "containers": {"foo": {}}} ) ctx.run( - container.pebble_ready_event(), + ctx.on.pebble_ready(container), state_in, ) assert local_file.read().decode() == "TEST" @@ -661,7 +661,7 @@ def test_pebble_push(): meta={"name": "foo", "containers": {"foo": {}}} ) - ctx.run("start", state_in) + ctx.run(ctx.on.start(), state_in) # This is the root of the simulated container filesystem. Any mounts will be symlinks in it. container_root_fs = container.get_filesystem(ctx) @@ -710,7 +710,7 @@ def test_pebble_exec(): meta={"name": "foo", "containers": {"foo": {}}}, ) state_out = ctx.run( - container.pebble_ready_event, + ctx.on.pebble_ready(container), state_in, ) ``` @@ -765,7 +765,7 @@ From test code, you can inspect that: import scenario ctx = scenario.Context(MyCharm) -ctx.run('some-event-that-will-cause_on_foo-to-be-called', scenario.State()) +ctx.run(ctx.on.some_event_that_will_cause_on_foo_to_be_called(), scenario.State()) # the charm has requested two 'foo' storages to be provisioned: assert ctx.requested_storages['foo'] == 2 @@ -780,11 +780,11 @@ import scenario ctx = scenario.Context(MyCharm) foo_0 = scenario.Storage('foo') # The charm is notified that one of the storages it has requested is ready: -ctx.run(foo_0.attached_event, State(storage=[foo_0])) +ctx.run(ctx.on.storage_sttached(foo_0), State(storage=[foo_0])) foo_1 = scenario.Storage('foo') # The charm is notified that the other storage is also ready: -ctx.run(foo_1.attached_event, State(storage=[foo_0, foo_1])) +ctx.run(ctx.on.storage_attached(foo_1), State(storage=[foo_0, foo_1])) ``` ## Ports @@ -796,17 +796,17 @@ Since `ops 2.6.0`, charms can invoke the `open-port`, `close-port`, and `opened- import scenario ctx = scenario.Context(MyCharm) -ctx.run("start", scenario.State(opened_ports=[scenario.Port("tcp", 42)])) +ctx.run(ctx.on.start(), scenario.State(opened_ports=[scenario.Port("tcp", 42)])) ``` - assert that a charm has called `open-port` or `close-port`: ```python import scenario ctx = scenario.Context(MyCharm) -state1 = ctx.run("start", scenario.State()) +state1 = ctx.run(ctx.on.start(), scenario.State()) assert state1.opened_ports == [scenario.Port("tcp", 42)] -state2 = ctx.run("stop", state1) +state2 = ctx.run(ctx.on.stop(), state1) assert state2.opened_ports == [] ``` @@ -944,7 +944,7 @@ class MyCharm(ops.CharmBase): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) state_in = scenario.State(model=scenario.Model(name="my-model")) -out = ctx.run("start", state_in) +out = ctx.run(ctx.on.start(), state_in) assert out.model.name == "my-model" assert out.model.uuid == state_in.model.uuid ``` @@ -1035,7 +1035,7 @@ def test_start_on_deferred_update_status(MyCharm): scenario.deferred('update_status', handler=MyCharm._on_update_status) ] ) - state_out = scenario.Context(MyCharm).run('start', state_in) + state_out = scenario.Context(MyCharm).run(ctx.on.start(), state_in) assert len(state_out.deferred) == 1 assert state_out.deferred[0].name == 'start' ``` @@ -1070,7 +1070,7 @@ class MyCharm(...): def test_defer(MyCharm): - out = scenario.Context(MyCharm).run('start', scenario.State()) + out = scenario.Context(MyCharm).run(ctx.on.start(), scenario.State()) assert len(out.deferred) == 1 assert out.deferred[0].name == 'start' ``` @@ -1198,7 +1198,7 @@ class MyCharmType(ops.CharmBase): ctx = scenario.Context(charm_type=MyCharmType, meta={'name': 'my-charm-name'}) -ctx.run('start', State()) +ctx.run(ctx.on.start(), State()) ``` A consequence of this fact is that you have no direct control over the temporary directory that we are creating to put the metadata @@ -1220,7 +1220,7 @@ state = scenario.Context( charm_type=MyCharmType, meta={'name': 'my-charm-name'}, charm_root=td.name -).run('start', scenario.State()) +).run(ctx.on.start(), scenario.State()) ``` Do this, and you will be able to set up said directory as you like before the charm is run, as well as verify its diff --git a/tests/test_charm_spec_autoload.py b/tests/test_charm_spec_autoload.py index 89a9cfdf..dec3214b 100644 --- a/tests/test_charm_spec_autoload.py +++ b/tests/test_charm_spec_autoload.py @@ -115,7 +115,7 @@ def test_meta_autoload(tmp_path, legacy): meta={"type": "charm", "name": "foo", "summary": "foo", "description": "foo"}, ) as charm: ctx = Context(charm) - ctx.run("start", State()) + ctx.run(ctx.on.start(), State()) @pytest.mark.parametrize("legacy", (True, False)) @@ -143,7 +143,7 @@ def test_relations_ok(tmp_path, legacy): }, ) as charm: # this would fail if there were no 'cuddles' relation defined in meta - Context(charm).run("start", State(relations=[Relation("cuddles")])) + Context(charm).run(ctx.on.start(), State(relations=[Relation("cuddles")])) @pytest.mark.parametrize("legacy", (True, False)) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index eaf89422..8b9a8401 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -61,22 +61,23 @@ def _on_event(self, event): def test_simple_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) # These look like: - # ctx.run(ctx.on.install, state) - with ctx.manager(getattr(ctx.on, event_name), scenario.State()) as mgr: + # ctx.run(ctx.on.install(), state) + with ctx.manager(getattr(ctx.on, event_name)(), scenario.State()) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) assert isinstance(mgr.charm.observed[0], event_kind) +@pytest.mark.parametrize("as_kwarg", [True, False]) @pytest.mark.parametrize( - "event_name, event_kind", + "event_name,event_kind", [ ("secret_changed", ops.SecretChangedEvent), ("secret_rotate", ops.SecretRotateEvent), ], ) -def test_simple_secret_events(event_name, event_kind): +def test_simple_secret_events(as_kwarg, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) secret = scenario.Secret("secret:123", {0: {"password": "xxxx"}}, owner=None) state_in = scenario.State(secrets=[secret]) @@ -84,7 +85,13 @@ def test_simple_secret_events(event_name, event_kind): # ctx.run(ctx.on.secret_changed(secret=secret), state) # The secret must always be passed because the same event name is used for # all secrets. - with ctx.manager(getattr(ctx.on, event_name)(secret=secret), state_in) as mgr: + if as_kwarg: + args = () + kwargs = {"secret": secret} + else: + args = (secret,) + kwargs = {} + with ctx.manager(getattr(ctx.on, event_name)(*args, **kwargs), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -111,7 +118,7 @@ def test_revision_secret_events(event_name, event_kind): # The secret and revision must always be passed because the same event name # is used for all secrets. with ctx.manager( - getattr(ctx.on, event_name)(secret=secret, revision=42), state_in + getattr(ctx.on, event_name)(secret, revision=42), state_in ) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 @@ -122,36 +129,69 @@ def test_revision_secret_events(event_name, event_kind): assert event.revision == 42 +@pytest.mark.parametrize("event_name", ["secret_expired", "secret_remove"]) +def test_revision_secret_events_as_positional_arg(event_name): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + secret = scenario.Secret( + "secret:123", {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, owner=None + ) + state_in = scenario.State(secrets=[secret]) + with pytest.assertRaises(ValueError): + ctx.run(getattr(ctx.on, event_name)(secret, 42), state_in) + + +@pytest.mark.parametrize("as_kwarg", [True, False]) +@pytest.mark.parametrize("storage_index", [0, 1, None]) @pytest.mark.parametrize( "event_name, event_kind", [ - ("foo_storage_attached", ops.StorageAttachedEvent), - ("foo_storage_detaching", ops.StorageDetachingEvent), + ("storage_attached", ops.StorageAttachedEvent), + ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(event_name, event_kind): +def test_storage_events(as_kwarg, storage_index, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storage = scenario.Storage("foo") - state_in = scenario.State(storage=[storage]) + storages = [scenario.Storage("foo", index) for index in range((storage_index or 0) + 1)] + state_in = scenario.State(storage=storages) # These look like: - # ctx.run(ctx.on.foo_storage_attached, state) - # The storage is inferred from the event name. - with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: + # ctx.run(ctx.on.storage_attached(storage), state) + if as_kwarg: + args = () + if storage_index is None: + kwargs = {"storage": storages[-1]} + else: + kwargs = {"storage": storages[-1], "index": storages[-1].index} + else: + args = (storages[-1],) + if storage_index is None: + kwargs = {} + else: + kwargs = {"index": storages[-1].index} + with ctx.manager(getattr(ctx.on, event_name)(*args, **kwargs), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storage.name - assert event.storage.index == storage.index + assert event.storage.name == storages[-1].name + assert event.storage.index == storages[-1].index + + +@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) +def test_storage_events_as_positional_arg(event_name): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) + with pytest.assertRaises(ValueError): + ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) def test_action_event_no_params(): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) # These look like: - # ctx.run_action(ctx.on.act_action, state) - # The action is inferred from the event name. - with ctx.action_manager(ctx.on.act_action, scenario.State()) as mgr: + # ctx.run_action(ctx.on.action(action), state) + action = scenario.Action("act") + with ctx.action_manager(ctx.on.action(action), scenario.State()) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -180,9 +220,8 @@ def test_pebble_ready_event(): container = scenario.Container("bar", can_connect=True) state_in = scenario.State(containers=[container]) # These look like: - # ctx.run(ctx.on.bar_pebble_ready, state) - # The container/workload is inferred from the event name. - with ctx.manager(ctx.on.bar_pebble_ready, state_in) as mgr: + # ctx.run(ctx.on.pebble_ready(container), state) + with ctx.manager(ctx.on.pebble_ready(container), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -191,21 +230,27 @@ def test_pebble_ready_event(): assert event.workload.name == container.name +@pytest.mark.parametrize("as_kwarg", [True, False]) @pytest.mark.parametrize( "event_name, event_kind", [ - ("baz_relation_created", ops.RelationCreatedEvent), - ("baz_relation_broken", ops.RelationBrokenEvent), + ("relation_created", ops.RelationCreatedEvent), + ("relation_broken", ops.RelationBrokenEvent), ], ) -def test_relation_app_events(event_name, event_kind): +def test_relation_app_events(as_kwarg, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) relation = scenario.Relation("baz") state_in = scenario.State(relations=[relation]) # These look like: - # ctx.run(ctx.on.baz_relation_created, state) - # The relation is inferred from the event name. - with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: + # ctx.run(ctx.on.relation_created(relation), state) + if as_kwarg: + args = () + kwargs = {"relation": relation} + else: + args = (relation,) + kwargs = {} + with ctx.manager(getattr(ctx.on, event_name)(*args, **kwargs), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -216,11 +261,20 @@ def test_relation_app_events(event_name, event_kind): assert event.unit is None +@pytest.mark.parametrize("event_name", ["relation_created", "relation_broken"]) +def test_relation_events_as_positional_arg(event_name): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + relation = scenario.Relation("baz") + state_in = scenario.State(relations=[relation]) + with pytest.assertRaises(ValueError): + ctx.run(getattr(ctx.on, event_name)(relation, 0), state_in) + + @pytest.mark.parametrize( "event_name, event_kind", [ - ("baz_relation_joined", ops.RelationJoinedEvent), - ("baz_relation_changed", ops.RelationChangedEvent), + ("relation_joined", ops.RelationJoinedEvent), + ("relation_changed", ops.RelationChangedEvent), ], ) def test_relation_unit_events_default_unit(event_name, event_kind): @@ -229,9 +283,8 @@ def test_relation_unit_events_default_unit(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed, state) - # The relation is inferred from the event name and the unit is chosen - # automatically. - with ctx.manager(getattr(ctx.on, event_name), state_in) as mgr: + # The unit is chosen automatically. + with ctx.manager(getattr(ctx.on, event_name)(relation), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -245,8 +298,8 @@ def test_relation_unit_events_default_unit(event_name, event_kind): @pytest.mark.parametrize( "event_name, event_kind", [ - ("baz_relation_joined", ops.RelationJoinedEvent), - ("baz_relation_changed", ops.RelationChangedEvent), + ("relation_joined", ops.RelationJoinedEvent), + ("relation_changed", ops.RelationChangedEvent), ], ) def test_relation_unit_events(event_name, event_kind): @@ -257,8 +310,7 @@ def test_relation_unit_events(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) - # The relation is inferred from the event name, and an explicit unit choice is provided. - with ctx.manager(getattr(ctx.on, event_name)(unit=2), state_in) as mgr: + with ctx.manager(getattr(ctx.on, event_name)(relation, unit=2), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -275,10 +327,8 @@ def test_relation_departed_event(): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_departed(unit=unit_ordinal, departing_unit=unit_ordinal), state) - # The relation is inferred from the event name, and an explicit unit choice is provided for - # both the triggering unit and the departing unit. with ctx.manager( - ctx.on.baz_relation_departed(unit=2, departing_unit=1), state_in + ctx.on.relation_departed(relation, unit=2, departing_unit=1), state_in ) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index 34dcf529..a87c89a7 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -196,10 +196,10 @@ def test_defer_reemit_lifecycle_event(mycharm): ctx = Context(mycharm, meta=mycharm.META, capture_deferred_events=True) mycharm.defer_next = 1 - state_1 = ctx.run("update-status", State()) + state_1 = ctx.run(ctx.on.update_status(), State()) mycharm.defer_next = 0 - state_2 = ctx.run("start", state_1) + state_2 = ctx.run(ctx.on.start(), state_1) assert [type(e).__name__ for e in ctx.emitted_events] == [ "UpdateStatusEvent", @@ -215,10 +215,10 @@ def test_defer_reemit_relation_event(mycharm): rel = Relation("foo") mycharm.defer_next = 1 - state_1 = ctx.run(rel.created_event, State(relations=[rel])) + state_1 = ctx.run(ctx.on.relation_created(rel), State(relations=[rel])) mycharm.defer_next = 0 - state_2 = ctx.run("start", state_1) + state_2 = ctx.run(ctx.on.start(), state_1) assert [type(e).__name__ for e in ctx.emitted_events] == [ "RelationCreatedEvent", diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index 0dd50077..d1ad29c5 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -61,7 +61,7 @@ class MyCharm(CharmBase): META = {"name": "joop"} ctx = Context(MyCharm, meta=MyCharm.META, capture_framework_events=True) - ctx.run("update-status", State()) + ctx.run(ctx.on.update_status(), State()) assert len(ctx.emitted_events) == 4 assert list(map(type, ctx.emitted_events)) == [ ops.UpdateStatusEvent, @@ -84,7 +84,7 @@ def _foo(self, e): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run("start", State(deferred=[Event("update-status").deferred(MyCharm._foo)])) + ctx.run(ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)])) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ diff --git a/tests/test_e2e/test_juju_log.py b/tests/test_e2e/test_juju_log.py index 5f58a973..e76c04bc 100644 --- a/tests/test_e2e/test_juju_log.py +++ b/tests/test_e2e/test_juju_log.py @@ -31,7 +31,7 @@ def _on_event(self, event): def test_juju_log(mycharm): ctx = Context(mycharm, meta=mycharm.META) - ctx.run("start", State()) + ctx.run(ctx.on.start(), State()) assert ctx.juju_log[-2] == JujuLogLine( level="DEBUG", message="Emitting Juju event start." ) diff --git a/tests/test_e2e/test_ports.py b/tests/test_e2e/test_ports.py index dc10b3a9..8343c976 100644 --- a/tests/test_e2e/test_ports.py +++ b/tests/test_e2e/test_ports.py @@ -27,7 +27,7 @@ def ctx(): def test_open_port(ctx): - out = ctx.run("start", State()) + out = ctx.run(ctx.start(), State()) port = out.opened_ports.pop() assert port.protocol == "tcp" @@ -35,5 +35,5 @@ def test_open_port(ctx): def test_close_port(ctx): - out = ctx.run("stop", State(opened_ports=[Port("tcp", 42)])) + out = ctx.run(ctx.on.stop(), State(opened_ports=[Port("tcp", 42)])) assert not out.opened_ports diff --git a/tests/test_e2e/test_secrets.py b/tests/test_e2e/test_secrets.py index 953fee4a..dfb7bb11 100644 --- a/tests/test_e2e/test_secrets.py +++ b/tests/test_e2e/test_secrets.py @@ -431,10 +431,10 @@ def _on_start(self, _): secret.grant(self.model.relations["bar"][0]) state = State(leader=leader, relations=[Relation("bar")]) - context = Context( + ctx = Context( GrantingCharm, meta={"name": "foo", "provides": {"bar": {"interface": "bar"}}} ) - context.run("start", state) + ctx.run(ctx.on.start(), state) def test_grant_nonowner(mycharm): diff --git a/tests/test_e2e/test_status.py b/tests/test_e2e/test_status.py index e587b406..43231597 100644 --- a/tests/test_e2e/test_status.py +++ b/tests/test_e2e/test_status.py @@ -61,7 +61,7 @@ def _on_update_status(self, _): meta={"name": "local"}, ) - out = ctx.run("update_status", State(leader=True)) + out = ctx.run(ctx.on.update_status(), State(leader=True)) assert out.unit_status == WaitingStatus("3") assert ctx.unit_status_history == [ @@ -94,7 +94,7 @@ def _on_update_status(self, _): ) out = ctx.run( - "update_status", + ctx.on.update_status(), State( leader=True, unit_status=ActiveStatus("foo"), @@ -131,9 +131,9 @@ def _on_update_status(self, _): meta={"name": "local"}, ) - out = ctx.run("install", State(leader=True)) - out = ctx.run("start", out) - out = ctx.run("update_status", out) + out = ctx.run(ctx.ȯn.install(), State(leader=True)) + out = ctx.run(ctx.on.start(), out) + out = ctx.run(ctx.on.update_status(), out) assert ctx.workload_version_history == ["1", "1.1"] assert out.workload_version == "1.2" diff --git a/tests/test_e2e/test_storage.py b/tests/test_e2e/test_storage.py index 87aa9370..ec9fa0fb 100644 --- a/tests/test_e2e/test_storage.py +++ b/tests/test_e2e/test_storage.py @@ -83,9 +83,9 @@ def test_storage_usage(storage_ctx): def test_storage_attached_event(storage_ctx): storage = Storage("foo") - storage_ctx.run(storage.attached_event, State(storage=[storage])) + storage_ctx.run(storage_ctx.on.storage_attached(storage), State(storage=[storage])) def test_storage_detaching_event(storage_ctx): storage = Storage("foo") - storage_ctx.run(storage.detaching_event, State(storage=[storage])) + storage_ctx.run(storage_ctx.on.storage_detaching(storage), State(storage=[storage])) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 06873f17..7145a344 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -21,7 +21,7 @@ def context(): return Context(charm_type=MyCharm, meta={"name": "foo"}) def test_sth(context): - context.run('start', State()) + context.run(ctx.on.start(), State()) """ ) From d6824ca33a478bc4a174bf04affcbe5531b1ffd2 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 4 Jun 2024 16:44:21 +1200 Subject: [PATCH 05/23] Style fixes. --- tests/test_context_on.py | 8 ++++---- tests/test_e2e/test_event.py | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 8b9a8401..1ba1e70a 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -117,9 +117,7 @@ def test_revision_secret_events(event_name, event_kind): # ctx.run(ctx.on.secret_expired(secret=secret, revision=revision), state) # The secret and revision must always be passed because the same event name # is used for all secrets. - with ctx.manager( - getattr(ctx.on, event_name)(secret, revision=42), state_in - ) as mgr: + with ctx.manager(getattr(ctx.on, event_name)(secret, revision=42), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -151,7 +149,9 @@ def test_revision_secret_events_as_positional_arg(event_name): ) def test_storage_events(as_kwarg, storage_index, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storages = [scenario.Storage("foo", index) for index in range((storage_index or 0) + 1)] + storages = [ + scenario.Storage("foo", index) for index in range((storage_index or 0) + 1) + ] state_in = scenario.State(storage=storages) # These look like: # ctx.run(ctx.on.storage_attached(storage), state) diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index d1ad29c5..40da6bce 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -84,7 +84,9 @@ def _foo(self, e): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run(ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)])) + ctx.run( + ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)]) + ) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ From 17cfe81a507f9a89adf75ae75b1e138f219d0e87 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 10:51:42 +1200 Subject: [PATCH 06/23] Fix tests. The failing test are (a) ones that need to be rewritten for the new system, (b) ones to do with custom events. --- .pre-commit-config.yaml | 2 +- README.md | 6 - scenario/__init__.py | 2 - scenario/consistency_checker.py | 22 +- scenario/context.py | 452 ++++++------------- scenario/mocking.py | 6 +- scenario/ops_main_mock.py | 15 +- scenario/runtime.py | 6 +- scenario/sequences.py | 33 +- scenario/state.py | 127 +----- tests/helpers.py | 10 +- tests/test_charm_spec_autoload.py | 6 +- tests/test_consistency_checker.py | 114 ++--- tests/test_context.py | 16 +- tests/test_context_on.py | 61 +-- tests/test_e2e/test_actions.py | 10 +- tests/test_e2e/test_config.py | 2 +- tests/test_e2e/test_custom_event_triggers.py | 146 ------ tests/test_e2e/test_deferred.py | 5 +- tests/test_e2e/test_event.py | 6 +- tests/test_e2e/test_event_bind.py | 19 +- tests/test_e2e/test_juju_log.py | 1 - tests/test_e2e/test_manager.py | 8 +- tests/test_e2e/test_network.py | 6 +- tests/test_e2e/test_pebble.py | 17 +- tests/test_e2e/test_ports.py | 2 +- tests/test_e2e/test_relations.py | 33 +- tests/test_e2e/test_rubbish_events.py | 4 +- tests/test_e2e/test_secrets.py | 75 +-- tests/test_e2e/test_status.py | 2 +- tests/test_e2e/test_storage.py | 14 +- tests/test_e2e/test_vroot.py | 10 +- tests/test_emitted_events_util.py | 7 +- tests/test_plugin.py | 2 +- tests/test_runtime.py | 10 +- 35 files changed, 432 insertions(+), 825 deletions(-) delete mode 100644 tests/test_e2e/test_custom_event_triggers.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f5fcd9e8..947fcf5c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,7 +13,7 @@ repos: - id: end-of-file-fixer - id: trailing-whitespace - repo: https://github.com/asottile/add-trailing-comma - rev: v2.4.0 + rev: v3.1.0 hooks: - id: add-trailing-comma args: [--py36-plus] diff --git a/README.md b/README.md index ba2f10c3..ff995d6b 100644 --- a/README.md +++ b/README.md @@ -1118,12 +1118,6 @@ foo_relation = scenario.Relation('foo') foo_relation.changed_event.deferred(handler=MyCharm._on_foo_relation_changed) ``` -# Emitting custom events - -Suppose your charm uses a charm library providing an `ingress_provided` event. -The proper way to emit it is to run the event that causes that custom event to be emitted by the library, whatever -that may be, for example a `foo-relation-changed`. - # Live charm introspection Scenario is a black-box, state-transition testing framework. It makes it trivial to assert that a status went from A to diff --git a/scenario/__init__.py b/scenario/__init__.py index b5e63a4c..a257ccc8 100644 --- a/scenario/__init__.py +++ b/scenario/__init__.py @@ -8,7 +8,6 @@ BindAddress, Container, DeferredEvent, - Event, ExecOutput, Model, Mount, @@ -47,5 +46,4 @@ "StoredState", "State", "DeferredEvent", - "Event", ] diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 72eb67dc..89002d67 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -20,7 +20,7 @@ ) if TYPE_CHECKING: # pragma: no cover - from scenario.state import Event, State + from scenario.state import State, _Event logger = scenario_logger.getChild("consistency_checker") @@ -34,7 +34,7 @@ class Results(NamedTuple): def check_consistency( state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", juju_version: str, ): @@ -119,7 +119,7 @@ def check_resource_consistency( def check_event_consistency( *, - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", **_kwargs, # noqa: U101 ) -> Results: @@ -157,7 +157,7 @@ def check_event_consistency( def _check_relation_event( charm_spec: _CharmSpec, # noqa: U100 - event: "Event", + event: "_Event", errors: List[str], warnings: List[str], # noqa: U100 ): @@ -176,7 +176,7 @@ def _check_relation_event( def _check_workload_event( charm_spec: _CharmSpec, # noqa: U100 - event: "Event", + event: "_Event", errors: List[str], warnings: List[str], # noqa: U100 ): @@ -194,7 +194,7 @@ def _check_workload_event( def _check_action_event( charm_spec: _CharmSpec, - event: "Event", + event: "_Event", errors: List[str], warnings: List[str], ): @@ -223,7 +223,7 @@ def _check_action_event( def _check_storage_event( charm_spec: _CharmSpec, - event: "Event", + event: "_Event", errors: List[str], warnings: List[str], # noqa: U100 ): @@ -396,7 +396,7 @@ def check_config_consistency( def check_secrets_consistency( *, - event: "Event", + event: "_Event", state: "State", juju_version: Tuple[int, ...], **_kwargs, # noqa: U101 @@ -422,7 +422,7 @@ def check_secrets_consistency( def check_network_consistency( *, state: "State", - event: "Event", # noqa: U100 + event: "_Event", # noqa: U100 charm_spec: "_CharmSpec", **_kwargs, # noqa: U101 ) -> Results: @@ -454,7 +454,7 @@ def check_network_consistency( def check_relation_consistency( *, state: "State", - event: "Event", # noqa: U100 + event: "_Event", # noqa: U100 charm_spec: "_CharmSpec", **_kwargs, # noqa: U101 ) -> Results: @@ -523,7 +523,7 @@ def _get_relations(r): def check_containers_consistency( *, state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", **_kwargs, # noqa: U101 ) -> Results: diff --git a/scenario/context.py b/scenario/context.py index 6e90523d..70bcff7b 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -5,18 +5,7 @@ import tempfile from contextlib import contextmanager from pathlib import Path -from typing import ( - TYPE_CHECKING, - Any, - Callable, - Dict, - List, - Optional, - Self, - Type, - Union, - cast, -) +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type, Union, cast from ops import CharmBase, EventBase @@ -25,11 +14,11 @@ from scenario.state import ( Action, Container, - Event, MetadataNotFoundError, Secret, Storage, _CharmSpec, + _Event, ) if TYPE_CHECKING: # pragma: no cover @@ -88,7 +77,7 @@ class _Manager: def __init__( self, ctx: "Context", - arg: Union[str, Action, Event], + arg: Union[str, Action, _Event], state_in: "State", ): self._ctx = ctx @@ -173,162 +162,149 @@ def _get_output(self): return self._ctx._finalize_action(self._ctx.output_state) # noqa -@dataclasses.dataclass -class _EventSource: - kind: str +class _CharmEvents: + """Events generated by Juju pertaining to application lifecycle. + By default, the events listed as attributes of this class will be + provided via the :attr:`Context.on` attribute. For example:: -@dataclasses.dataclass -class _SecretEventSource(_EventSource): - _secret: Optional[Secret] = None - _revision: Optional[int] = None - - def __call__(self, secret: Secret, revision: Optional[int] = None) -> Self: - """Link to a specific scenario.Secret object.""" - self._secret = secret - self._revision = revision - return self + ctx.run(ctx.on.config_changed(), state) + This behaves similarly to the :class:`ops.CharmEvents` class but is much + simpler as there are no dynamically named attributes, and no __getattr__ + version to get events. In addition, all of the attributes are methods, + which are used to connect the event to the specific container object that + they relate to (or, for simpler events like "start" or "stop", take no + arguments). + """ -@dataclasses.dataclass -class _RelationEventSource(_EventSource): - name: str - _unit_id: Optional[str] = None - _departing_unit_id: Optional[str] = None + # TODO: There are lots of suffix definitions in state - we should be able to re-use those. - def __call__( - self, - unit: Optional[str] = None, - departing_unit: Optional[str] = None, - ) -> Self: - self._unit_id = unit - self._departing_unit_id = departing_unit - return self + @staticmethod + def install(): + return _Event("install") + @staticmethod + def start(): + return _Event("start") -@dataclasses.dataclass -class _StorageEventSource(_EventSource): - name: str + @staticmethod + def stop(): + return _Event("stop") + @staticmethod + def remove(): + return _Event("remove") -@dataclasses.dataclass -class _ContainerEventSource(_EventSource): - name: str + @staticmethod + def update_status(): + return _Event("update_status") + @staticmethod + def config_changed(): + return _Event("config_changed") -@dataclasses.dataclass -class _ActionEventSource(_EventSource): - name: str - _action: Optional[Action] = None - - def __call__(self, action: Action) -> Self: - """Provide a scenario.Action object, in order to specify params or id.""" - if action.name != self.name: - raise RuntimeError("WRITE AN ERROR MESSAGE HERE") - self._action = action - return self + @staticmethod + def upgrade_charm(): + return _Event("upgrade_charm") + @staticmethod + def pre_series_upgrade(): + return _Event("pre_series_upgrade") -class _CharmEvents: - """Events generated by Juju pertaining to application lifecycle. + @staticmethod + def post_series_upgrade(): + return _Event("post_series_upgrade") - By default, the events listed as attributes of this class will be - provided via the :attr:`Context.on` attribute. For example:: + @staticmethod + def leader_elected(): + return _Event("leader_elected") - ctx.run(ctx.on.config_changed, state) + @staticmethod + def secret_changed(secret: Secret): + return _Event("secret_changed", secret=secret) - In addition to the events listed as attributes of this class, - dynamically-named events will also be defined based on the charm's - metadata for relations, storage, actions, and containers. These named - events may be accessed as ``ctx.on._``, for example:: + @staticmethod + def secret_expired(secret: Secret, *, revision: int): + # TODO: Could we have a default revision? + return _Event("secret_expired", secret=secret, secret_revision=revision) - ctx.run(ctx.on.workload_pebble_ready, state) + @staticmethod + def secret_rotate(secret: Secret = None): + return _Event("secret_rotate", secret=secret) - See also the :class:`ops.CharmEvents` class. - """ + @staticmethod + def secret_remove(secret: Secret, *, revision: int): + # TODO: Could we have a default revision? + return _Event("secret_remove", secret=secret, secret_revision=revision) - # TODO: There are lots of suffix definitions in state - we should be able to re-use those. - install = _EventSource("install") - start = _EventSource("start") - stop = _EventSource("stop") - remove = _EventSource("remove") - update_status = _EventSource("update_status") - config_changed = _EventSource("config_changed") - upgrade_charm = _EventSource("upgrade_charm") - pre_series_upgrade = _EventSource("pre_series_upgrade") - post_series_upgrade = _EventSource("post_series_upgrade") - leader_elected = _EventSource("leader_elected") - secret_changed = _SecretEventSource("secret_changed") - secret_expired = _SecretEventSource("secret_expired") - secret_rotate = _SecretEventSource("secret_rotate") - secret_remove = _SecretEventSource("secret_remove") - collect_app_status = _EventSource("collect_app_status") - collect_unit_status = _EventSource("collect_unit_status") - - def __init__(self, charm_spec: _CharmSpec): - for relation_name, _ in charm_spec.get_all_relations(): - relation_name = relation_name.replace("-", "_") - setattr( - self, - f"{relation_name}_relation_created", - _RelationEventSource("relation_created", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_joined", - _RelationEventSource("relation_joined", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_changed", - _RelationEventSource("relation_changed", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_departed", - _RelationEventSource("relation_departed", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_broken", - _RelationEventSource("relation_broken", relation_name), - ) + @staticmethod + def collect_app_status(): + return _Event("collect_app_status") - for storage_name in charm_spec.meta.get("storage", ()): - storage_name = storage_name.replace("-", "_") - setattr( - self, - f"{storage_name}_storage_attached", - _StorageEventSource("storage_attached", storage_name), - ) - setattr( - self, - f"{storage_name}_storage_detaching", - _StorageEventSource("storage_detaching", storage_name), - ) + @staticmethod + def collect_unit_status(): + return _Event("collect_unit_status") - for action_name in charm_spec.actions or (): - action_name = action_name.replace("-", "_") - setattr( - self, - f"{action_name}_action", - _ActionEventSource("action", action_name), - ) + @staticmethod + def relation_created(relation: "AnyRelation", *, remote_unit: Optional[int] = None): + # TODO: Does remote_unit make sense here? The old Scenario API had it. + return _Event( + f"{relation.endpoint}_relation_created", + relation=relation, + relation_remote_unit_id=remote_unit, + ) - for container_name in charm_spec.meta.get("containers", ()): - container_name = container_name.replace("-", "_") - setattr( - self, - f"{container_name}_pebble_ready", - _ContainerEventSource("pebble_ready", container_name), - ) + @staticmethod + def relation_joined(relation: "AnyRelation", *, remote_unit: Optional[int] = None): + return _Event( + f"{relation.endpoint}_relation_joined", + relation=relation, + relation_remote_unit_id=remote_unit, + ) + @staticmethod + def relation_changed(relation: "AnyRelation", *, remote_unit: Optional[int] = None): + return _Event( + f"{relation.endpoint}_relation_changed", + relation=relation, + relation_remote_unit_id=remote_unit, + ) -# setattr( -# self, -# f"{container_name}_pebble_custom_notice", -# _ContainerEventSource("pebble_custom_notice", container_name), -# ) + @staticmethod + def relation_departed( + relation: "AnyRelation", + *, + remote_unit: Optional[int] = None, + departing_unit: Optional[int] = None, + ): + return _Event( + f"{relation.endpoint}_relation_departed", + relation=relation, + relation_remote_unit_id=remote_unit, + relation_departed_unit_id=departing_unit, + ) + + @staticmethod + def relation_broken(relation: "AnyRelation", *, remote_unit: Optional[int] = None): + # TODO: Does remote_unit make sense here? The old Scenario API had it. + return _Event( + f"{relation.endpoint}_relation_broken", + relation=relation, + relation_remote_unit_id=remote_unit, + ) + + @staticmethod + def storage_attached(storage: Storage): + return _Event(f"{storage.name}_storage_attached", storage=storage) + + @staticmethod + def storage_detaching(storage: Storage): + return _Event(f"{storage.name}_storage_detaching", storage=storage) + + @staticmethod + def pebble_ready(container: Container): + return _Event(f"{container.name}_pebble_ready", container=container) class Context: @@ -384,7 +360,7 @@ def __init__( >>> # Arrange: set the context up >>> c = Context(MyCharm) >>> # Act: prepare the state and emit an event - >>> state_out = c.run('update-status', State()) + >>> state_out = c.run(c.update_status(), State()) >>> # Assert: verify the output state is what you think it should be >>> assert state_out.unit_status == ActiveStatus('foobar') >>> # Assert: verify the Context contains what you think it should @@ -468,7 +444,7 @@ def __init__( self._action_results: Optional[Dict[str, str]] = None self._action_failure: Optional[str] = None - self.on = _CharmEvents(self.charm_spec) + self.on = _CharmEvents() def _set_output_state(self, output_state: "State"): """Hook for Runtime to set the output state.""" @@ -504,203 +480,68 @@ def _record_status(self, state: "State", is_app: bool): else: self.unit_status_history.append(cast("_EntityStatus", state.unit_status)) - @staticmethod - def _coalesce_action(action: Union[str, Action, _ActionEventSource]) -> Action: - """Validate the action argument and cast to Action.""" - if isinstance(action, str): - return Action(action) - - if isinstance(action, _ActionEventSource): - if action._action: - return action._action - return Action(action.name) - - if not isinstance(action, Action): - raise InvalidActionError( - f"Expected Action or action name; got {type(action)}", - ) - return action - - # TODO: These don't really need to be List, probably Iterable is fine? Really ought to be Mapping ... - @staticmethod - def _coalesce_event( - event: Union[str, Event, _EventSource], - *, - containers: Optional[List[Container]] = None, - storages: Optional[List[Storage]] = None, - relations: Optional[List["AnyRelation"]] = None, - ) -> Event: - """Validate the event argument and cast to Event.""" - if isinstance(event, str): - event = Event(event) - - if isinstance(event, _EventSource): - if event.kind == "action": - raise InvalidEventError( - "Cannot Context.run() action events. " - "Use Context.run_action instead.", - ) - if isinstance(event, _SecretEventSource): - if (secret := event._secret) is None: - raise InvalidEventError( - "A secret must be provided, for example: " - "ctx.run(ctx.on.secret_changed(secret=secret), state)", - ) - else: - secret = None - secret_revision = getattr(event, "_revision", None) - # TODO: These can probably do Event.bind() - if isinstance(event, _StorageEventSource): - # TODO: It would be great if this was a mapping, not a list. - for storage in storages or (): - if storage.name == event.name: - break - else: - raise InvalidEventError( - f"Attempting to run {event.name}_{event.kind}, but " - f"{event.name} is not a storage in the state.", - ) - else: - storage = None - if isinstance(event, _ContainerEventSource): - # TODO: It would be great if this was a mapping, not a list. - for container in containers or (): - if container.name == event.name: - break - else: - raise InvalidEventError( - f"Attempting to run {event.name}_{event.kind}, but " - f"{event.name} is not a container in the state.", - ) - else: - container = None - if isinstance(event, _RelationEventSource): - # TODO: It would be great if this was a mapping, not a list. - for relation in relations or (): - if relation.endpoint == event.name: - break - else: - raise InvalidEventError( - f"Attempting to run {event.name}_{event.kind}, but " - f"{event.name} is not a relation in the state.", - ) - else: - relation = None - relation_remote_unit_id = getattr(event, "_unit_id", None) - relation_departed_unit_id = getattr(event, "_departing_unit_id", None) - if hasattr(event, "name"): - path = f"{event.name}_{event.kind}" # type: ignore - else: - path = event.kind - event = Event( - path, - secret=secret, - secret_revision=secret_revision, - storage=storage, - container=container, - relation=relation, - relation_remote_unit_id=relation_remote_unit_id, - relation_departed_unit_id=relation_departed_unit_id, - ) - - if not isinstance(event, Event): - raise InvalidEventError(f"Expected Event | str, got {type(event)}") - - if event._is_action_event: # noqa - raise InvalidEventError( - "Cannot Context.run() action events. " - "Use Context.run_action instead.", - ) - return event - - def manager( - self, - event: Union["Event", str], - state: "State", - ): + def manager(self, event: "_Event", state: "State"): """Context manager to introspect live charm object before and after the event is emitted. Usage: - >>> with Context().manager("start", State()) as manager: + >>> with Context().manager(ctx.on.start(), State()) as manager: >>> assert manager.charm._some_private_attribute == "foo" # noqa >>> manager.run() # this will fire the event >>> assert manager.charm._some_private_attribute == "bar" # noqa - :arg event: the Event that the charm will respond to. Can be a string or an Event instance. + :arg event: the Event that the charm will respond to. :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Event. """ return _EventManager(self, event, state) - def action_manager( - self, - action: Union["Action", str], - state: "State", - ): + def action_manager(self, action: "Action", state: "State"): """Context manager to introspect live charm object before and after the event is emitted. Usage: - >>> with Context().action_manager("foo-action", State()) as manager: + >>> with Context().action_manager(Action("foo"), State()) as manager: >>> assert manager.charm._some_private_attribute == "foo" # noqa >>> manager.run() # this will fire the event >>> assert manager.charm._some_private_attribute == "bar" # noqa - :arg action: the Action that the charm will execute. Can be a string or an Action instance. + :arg action: the Action that the charm will execute. :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Action (event). """ return _ActionManager(self, action, state) @contextmanager - def _run_event( - self, - event: Union["_EventSource", "Event", str], - state: "State", - ): - _event = self._coalesce_event( - event, - containers=state.containers, - storages=state.storage, - relations=state.relations, - ) - with self._run(event=_event, state=state) as ops: + def _run_event(self, event: "_Event", state: "State"): + with self._run(event=event, state=state) as ops: yield ops - def run( - self, - event: Union["_EventSource", "Event", str], - state: "State", - ) -> "State": + def run(self, event: "_Event", state: "State") -> "State": """Trigger a charm execution with an Event and a State. Calling this function will call ``ops.main`` and set up the context according to the specified ``State``, then emit the event on the charm. - :arg event: the Event that the charm will respond to. Can be a string or an Event instance - or an _EventSource. + :arg event: the Event that the charm will respond to. :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Event. """ + if isinstance(event, Action) or event.action: + raise InvalidEventError("Use run_action() to run an action event.") with self._run_event(event=event, state=state) as ops: ops.emit() return self.output_state - def run_action( - self, - action: Union["_ActionEventSource", "Action", str], - state: "State", - ) -> ActionOutput: + def run_action(self, action: "Action", state: "State") -> ActionOutput: """Trigger a charm execution with an Action and a State. Calling this function will call ``ops.main`` and set up the context according to the specified ``State``, then emit the event on the charm. - :arg action: the Action that the charm will execute. Can be a string or an Action instance. + :arg action: the Action that the charm will execute. :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Action (event). """ - _action = self._coalesce_action(action) - with self._run_action(action=_action, state=state) as ops: + with self._run_action(action=action, state=state) as ops: ops.emit() return self._finalize_action(self.output_state) @@ -720,21 +561,12 @@ def _finalize_action(self, state_out: "State"): return ao @contextmanager - def _run_action( - self, - action: Union["_ActionEventSource", "Action", str], - state: "State", - ): - _action = self._coalesce_action(action) - with self._run(event=_action.event, state=state) as ops: + def _run_action(self, action: "Action", state: "State"): + with self._run(event=action.event, state=state) as ops: yield ops @contextmanager - def _run( - self, - event: "Event", - state: "State", - ): + def _run(self, event: "_Event", state: "State"): runtime = Runtime( charm_spec=self.charm_spec, juju_version=self.juju_version, diff --git a/scenario/mocking.py b/scenario/mocking.py index 841ca25a..2ff13f5b 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -49,13 +49,13 @@ from scenario.context import Context from scenario.state import Container as ContainerSpec from scenario.state import ( - Event, ExecOutput, Relation, Secret, State, SubordinateRelation, _CharmSpec, + _Event, ) logger = scenario_logger.getChild("mocking") @@ -102,7 +102,7 @@ class _MockModelBackend(_ModelBackend): def __init__( self, state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", context: "Context", ): @@ -638,7 +638,7 @@ def __init__( mounts: Dict[str, Mount], *, state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", ): self._state = state diff --git a/scenario/ops_main_mock.py b/scenario/ops_main_mock.py index b25c7e1c..7aeaa60a 100644 --- a/scenario/ops_main_mock.py +++ b/scenario/ops_main_mock.py @@ -20,7 +20,7 @@ if TYPE_CHECKING: # pragma: no cover from scenario.context import Context - from scenario.state import Event, State, _CharmSpec + from scenario.state import State, _CharmSpec, _Event # pyright: reportPrivateUsage=false @@ -54,7 +54,7 @@ def _get_owner(root: Any, path: Sequence[str]) -> ops.ObjectEvents: def _emit_charm_event( charm: "CharmBase", event_name: str, - event: Optional["Event"] = None, + event: Optional["_Event"] = None, ): """Emits a charm event based on a Juju event name. @@ -83,7 +83,7 @@ def _emit_charm_event( def setup_framework( charm_dir, state: "State", - event: "Event", + event: "_Event", context: "Context", charm_spec: "_CharmSpec", ): @@ -154,7 +154,12 @@ def setup_charm(charm_class, framework, dispatcher): return charm -def setup(state: "State", event: "Event", context: "Context", charm_spec: "_CharmSpec"): +def setup( + state: "State", + event: "_Event", + context: "Context", + charm_spec: "_CharmSpec", +): """Setup dispatcher, framework and charm objects.""" charm_class = charm_spec.charm_type charm_dir = _get_charm_dir() @@ -173,7 +178,7 @@ class Ops: def __init__( self, state: "State", - event: "Event", + event: "_Event", context: "Context", charm_spec: "_CharmSpec", ): diff --git a/scenario/runtime.py b/scenario/runtime.py index 4fe5d59a..b65cf641 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -25,7 +25,7 @@ from ops.testing import CharmType from scenario.context import Context - from scenario.state import Event, State, _CharmSpec + from scenario.state import State, _CharmSpec, _Event PathLike = Union[str, Path] @@ -180,7 +180,7 @@ def _cleanup_env(env): # os.unsetenv does not always seem to work !? del os.environ[key] - def _get_event_env(self, state: "State", event: "Event", charm_root: Path): + def _get_event_env(self, state: "State", event: "_Event", charm_root: Path): """Build the simulated environment the operator framework expects.""" env = { "JUJU_VERSION": self._juju_version, @@ -392,7 +392,7 @@ def _exec_ctx(self, ctx: "Context"): def exec( self, state: "State", - event: "Event", + event: "_Event", context: "Context", ): """Runs an event with this state as initial state on a charm. diff --git a/scenario/sequences.py b/scenario/sequences.py index 2e1cbb22..5ebb7601 100644 --- a/scenario/sequences.py +++ b/scenario/sequences.py @@ -14,8 +14,8 @@ BREAK_ALL_RELATIONS, CREATE_ALL_RELATIONS, DETACH_ALL_STORAGES, - Event, State, + _Event, ) if typing.TYPE_CHECKING: # pragma: no cover @@ -25,7 +25,7 @@ logger = scenario_logger.getChild("scenario") -def decompose_meta_event(meta_event: Event, state: State): +def decompose_meta_event(meta_event: _Event, state: State): # decompose the meta event if meta_event.name in [ATTACH_ALL_STORAGES, DETACH_ALL_STORAGES]: @@ -50,15 +50,18 @@ def decompose_meta_event(meta_event: Event, state: State): def generate_startup_sequence(state_template: State): yield from chain( - decompose_meta_event(Event(ATTACH_ALL_STORAGES), copy.deepcopy(state_template)), - ((Event("start"), copy.deepcopy(state_template)),), decompose_meta_event( - Event(CREATE_ALL_RELATIONS), + _Event(ATTACH_ALL_STORAGES), + copy.deepcopy(state_template), + ), + ((_Event("start"), copy.deepcopy(state_template)),), + decompose_meta_event( + _Event(CREATE_ALL_RELATIONS), copy.deepcopy(state_template), ), ( ( - Event( + _Event( ( "leader_elected" if state_template.leader @@ -67,19 +70,25 @@ def generate_startup_sequence(state_template: State): ), copy.deepcopy(state_template), ), - (Event("config_changed"), copy.deepcopy(state_template)), - (Event("install"), copy.deepcopy(state_template)), + (_Event("config_changed"), copy.deepcopy(state_template)), + (_Event("install"), copy.deepcopy(state_template)), ), ) def generate_teardown_sequence(state_template: State): yield from chain( - decompose_meta_event(Event(BREAK_ALL_RELATIONS), copy.deepcopy(state_template)), - decompose_meta_event(Event(DETACH_ALL_STORAGES), copy.deepcopy(state_template)), + decompose_meta_event( + _Event(BREAK_ALL_RELATIONS), + copy.deepcopy(state_template), + ), + decompose_meta_event( + _Event(DETACH_ALL_STORAGES), + copy.deepcopy(state_template), + ), ( - (Event("stop"), copy.deepcopy(state_template)), - (Event("remove"), copy.deepcopy(state_template)), + (_Event("stop"), copy.deepcopy(state_template)), + (_Event("remove"), copy.deepcopy(state_template)), ), ) diff --git a/scenario/state.py b/scenario/state.py index 65a83e7f..a08163ac 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -118,10 +118,6 @@ class MetadataNotFoundError(RuntimeError): """Raised when Scenario can't find a metadata.yaml file in the provided charm root.""" -class BindFailedError(RuntimeError): - """Raised when Event.bind fails.""" - - @dataclasses.dataclass(frozen=True) class Secret: id: str @@ -157,7 +153,7 @@ def changed_event(self): raise ValueError( "This unit will never receive secret-changed for a secret it owns.", ) - return Event("secret_changed", secret=self) + return _Event("secret_changed", secret=self) # owner-only events @property @@ -167,7 +163,7 @@ def rotate_event(self): raise ValueError( "This unit will never receive secret-rotate for a secret it does not own.", ) - return Event("secret_rotate", secret=self) + return _Event("secret_rotate", secret=self) @property def expired_event(self): @@ -176,7 +172,7 @@ def expired_event(self): raise ValueError( "This unit will never receive secret-expire for a secret it does not own.", ) - return Event("secret_expire", secret=self) + return _Event("secret_expire", secret=self) @property def remove_event(self): @@ -185,7 +181,7 @@ def remove_event(self): raise ValueError( "This unit will never receive secret-removed for a secret it does not own.", ) - return Event("secret_removed", secret=self) + return _Event("secret_removed", secret=self) def _set_revision(self, revision: int): """Set a new tracked revision.""" @@ -363,41 +359,41 @@ def _validate_databag(self, databag: dict): ) @property - def changed_event(self) -> "Event": + def changed_event(self) -> "_Event": """Sugar to generate a -relation-changed event.""" - return Event( + return _Event( path=normalize_name(self.endpoint + "-relation-changed"), relation=cast("AnyRelation", self), ) @property - def joined_event(self) -> "Event": + def joined_event(self) -> "_Event": """Sugar to generate a -relation-joined event.""" - return Event( + return _Event( path=normalize_name(self.endpoint + "-relation-joined"), relation=cast("AnyRelation", self), ) @property - def created_event(self) -> "Event": + def created_event(self) -> "_Event": """Sugar to generate a -relation-created event.""" - return Event( + return _Event( path=normalize_name(self.endpoint + "-relation-created"), relation=cast("AnyRelation", self), ) @property - def departed_event(self) -> "Event": + def departed_event(self) -> "_Event": """Sugar to generate a -relation-departed event.""" - return Event( + return _Event( path=normalize_name(self.endpoint + "-relation-departed"), relation=cast("AnyRelation", self), ) @property - def broken_event(self) -> "Event": + def broken_event(self) -> "_Event": """Sugar to generate a -relation-broken event.""" - return Event( + return _Event( path=normalize_name(self.endpoint + "-relation-broken"), relation=cast("AnyRelation", self), ) @@ -668,7 +664,7 @@ def pebble_ready_event(self): "you **can** fire pebble-ready while the container cannot connect, " "but that's most likely not what you want.", ) - return Event(path=normalize_name(self.name + "-pebble-ready"), container=self) + return _Event(path=normalize_name(self.name + "-pebble-ready"), container=self) _RawStatusLiteral = Literal[ @@ -797,17 +793,17 @@ def get_filesystem(self, ctx: "Context") -> Path: return ctx._get_storage_root(self.name, self.index) @property - def attached_event(self) -> "Event": + def attached_event(self) -> "_Event": """Sugar to generate a -storage-attached event.""" - return Event( + return _Event( path=normalize_name(self.name + "-storage-attached"), storage=self, ) @property - def detaching_event(self) -> "Event": + def detaching_event(self) -> "_Event": """Sugar to generate a -storage-detached event.""" - return Event( + return _Event( path=normalize_name(self.name + "-storage-detaching"), storage=self, ) @@ -1135,7 +1131,7 @@ def _get_suffix_and_type(s: str) -> Tuple[str, _EventType]: @dataclasses.dataclass(frozen=True) -class Event: +class _Event: path: str args: Tuple[Any, ...] = () kwargs: Dict[str, Any] = dataclasses.field(default_factory=dict) @@ -1160,21 +1156,8 @@ class Event: # if this is an action event, the Action instance action: Optional["Action"] = None - # todo add other meta for - # - secret events - # - pebble? - # - action? - _owner_path: List[str] = dataclasses.field(default_factory=list) - def __call__(self, remote_unit_id: Optional[int] = None) -> "Event": - if remote_unit_id and not self._is_relation_event: - raise ValueError( - "cannot pass param `remote_unit_id` to a " - "non-relation event constructor.", - ) - return dataclasses.replace(self, relation_remote_unit_id=remote_unit_id) - def __post_init__(self): path = _EventPath(self.path) # bypass frozen dataclass @@ -1252,68 +1235,6 @@ def _is_builtin_event(self, charm_spec: "_CharmSpec"): # assuming it is owned by the charm, LOOKS LIKE that of a builtin event or not. return self._path.type is not _EventType.custom - def bind(self, state: State): - """Attach to this event the state component it needs. - - For example, a relation event initialized without a Relation instance will search for - a suitable relation in the provided state and return a copy of itself with that - relation attached. - - In case of ambiguity (e.g. multiple relations found on 'foo' for event - 'foo-relation-changed', we pop a warning and bind the first one. Use with care! - """ - entity_name = self._path.prefix - - if self._is_workload_event and not self.container: - try: - container = state.get_container(entity_name) - except ValueError: - raise BindFailedError(f"no container found with name {entity_name}") - return dataclasses.replace(self, container=container) - - if self._is_secret_event and not self.secret: - if len(state.secrets) < 1: - raise BindFailedError(f"no secrets found in state: cannot bind {self}") - if len(state.secrets) > 1: - raise BindFailedError( - f"too many secrets found in state: cannot automatically bind {self}", - ) - return dataclasses.replace(self, secret=state.secrets[0]) - - if self._is_storage_event and not self.storage: - storages = state.get_storages(entity_name) - if len(storages) < 1: - raise BindFailedError( - f"no storages called {entity_name} found in state", - ) - if len(storages) > 1: - logger.warning( - f"too many storages called {entity_name}: binding to first one", - ) - storage = storages[0] - return dataclasses.replace(self, storage=storage) - - if self._is_relation_event and not self.relation: - ep_name = entity_name - relations = state.get_relations(ep_name) - if len(relations) < 1: - raise BindFailedError(f"no relations on {ep_name} found in state") - if len(relations) > 1: - logger.warning(f"too many relations on {ep_name}: binding to first one") - return dataclasses.replace(self, relation=relations[0]) - - if self._is_action_event and not self.action: - raise BindFailedError( - "cannot automatically bind action events: if the action has mandatory parameters " - "this would probably result in horrible, undebuggable failures downstream.", - ) - - else: - raise BindFailedError( - f"cannot bind {self}: only relation, secret, " - f"or workload events can be bound.", - ) - def deferred(self, handler: Callable, event_id: int = 1) -> DeferredEvent: """Construct a DeferredEvent from this Event.""" handler_repr = repr(handler) @@ -1389,13 +1310,13 @@ class Action: the rare cases where a specific ID is required.""" @property - def event(self) -> Event: + def event(self) -> _Event: """Helper to generate an action event from this action.""" - return Event(self.name + ACTION_EVENT_SUFFIX, action=self) + return _Event(self.name + ACTION_EVENT_SUFFIX, action=self) def deferred( - event: Union[str, Event], + event: Union[str, _Event], handler: Callable, event_id: int = 1, relation: Optional["Relation"] = None, @@ -1403,5 +1324,5 @@ def deferred( ): """Construct a DeferredEvent from an Event or an event name.""" if isinstance(event, str): - event = Event(event, relation=relation, container=container) + event = _Event(event, relation=relation, container=container) return event.deferred(handler=handler, event_id=event_id) diff --git a/tests/helpers.py b/tests/helpers.py index 64a747d6..999c35e7 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -20,7 +20,7 @@ if TYPE_CHECKING: # pragma: no cover from ops.testing import CharmType - from scenario.state import Event, State + from scenario.state import State, _Event _CT = TypeVar("_CT", bound=Type[CharmType]) @@ -31,7 +31,7 @@ def trigger( state: "State", - event: Union["Event", str], + event: Union[str, "_Event"], charm_type: Type["CharmType"], pre_event: Optional[Callable[["CharmType"], None]] = None, post_event: Optional[Callable[["CharmType"], None]] = None, @@ -49,6 +49,12 @@ def trigger( charm_root=charm_root, juju_version=juju_version, ) + if isinstance(event, str): + if event.startswith("relation_"): + assert len(state.relations) == 1, "shortcut only works with one relation" + event = getattr(ctx.on, event)(state.relations[0]) + else: + event = getattr(ctx.on, event)() with ctx.manager(event, state=state) as mgr: if pre_event: pre_event(mgr.charm) diff --git a/tests/test_charm_spec_autoload.py b/tests/test_charm_spec_autoload.py index dec3214b..51ba1391 100644 --- a/tests/test_charm_spec_autoload.py +++ b/tests/test_charm_spec_autoload.py @@ -143,7 +143,8 @@ def test_relations_ok(tmp_path, legacy): }, ) as charm: # this would fail if there were no 'cuddles' relation defined in meta - Context(charm).run(ctx.on.start(), State(relations=[Relation("cuddles")])) + ctx = Context(charm) + ctx.run(ctx.on.start(), State(relations=[Relation("cuddles")])) @pytest.mark.parametrize("legacy", (True, False)) @@ -160,6 +161,7 @@ def test_config_defaults(tmp_path, legacy): config={"options": {"foo": {"type": "bool", "default": True}}}, ) as charm: # this would fail if there were no 'cuddles' relation defined in meta - with Context(charm).manager("start", State()) as mgr: + ctx = Context(charm) + with ctx.manager(ctx.on.start(), State()) as mgr: mgr.run() assert mgr.charm.config["foo"] is True diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 9d3c7fcf..2d63c4de 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -9,7 +9,6 @@ RELATION_EVENTS_SUFFIX, Action, Container, - Event, Network, PeerRelation, Relation, @@ -19,6 +18,7 @@ StoredState, SubordinateRelation, _CharmSpec, + _Event, ) @@ -28,7 +28,7 @@ class MyCharm(CharmBase): def assert_inconsistent( state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", juju_version="3.0", ): @@ -38,7 +38,7 @@ def assert_inconsistent( def assert_consistent( state: "State", - event: "Event", + event: "_Event", charm_spec: "_CharmSpec", juju_version="3.0", ): @@ -47,7 +47,7 @@ def assert_consistent( def test_base(): state = State() - event = Event("update_status") + event = _Event("update_status") spec = _CharmSpec(MyCharm, {}) assert_consistent(state, event, spec) @@ -55,12 +55,12 @@ def test_base(): def test_workload_event_without_container(): assert_inconsistent( State(), - Event("foo-pebble-ready", container=Container("foo")), + _Event("foo-pebble-ready", container=Container("foo")), _CharmSpec(MyCharm, {}), ) assert_consistent( State(containers=[Container("foo")]), - Event("foo-pebble-ready", container=Container("foo")), + _Event("foo-pebble-ready", container=Container("foo")), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) @@ -68,12 +68,12 @@ def test_workload_event_without_container(): def test_container_meta_mismatch(): assert_inconsistent( State(containers=[Container("bar")]), - Event("foo"), + _Event("foo"), _CharmSpec(MyCharm, {"containers": {"baz": {}}}), ) assert_consistent( State(containers=[Container("bar")]), - Event("foo"), + _Event("foo"), _CharmSpec(MyCharm, {"containers": {"bar": {}}}), ) @@ -81,12 +81,12 @@ def test_container_meta_mismatch(): def test_container_in_state_but_no_container_in_meta(): assert_inconsistent( State(containers=[Container("bar")]), - Event("foo"), + _Event("foo"), _CharmSpec(MyCharm, {}), ) assert_consistent( State(containers=[Container("bar")]), - Event("foo"), + _Event("foo"), _CharmSpec(MyCharm, {"containers": {"bar": {}}}), ) @@ -94,12 +94,12 @@ def test_container_in_state_but_no_container_in_meta(): def test_evt_bad_container_name(): assert_inconsistent( State(), - Event("foo-pebble-ready", container=Container("bar")), + _Event("foo-pebble-ready", container=Container("bar")), _CharmSpec(MyCharm, {}), ) assert_consistent( State(containers=[Container("bar")]), - Event("bar-pebble-ready", container=Container("bar")), + _Event("bar-pebble-ready", container=Container("bar")), _CharmSpec(MyCharm, {"containers": {"bar": {}}}), ) @@ -108,22 +108,22 @@ def test_evt_bad_container_name(): def test_evt_bad_relation_name(suffix): assert_inconsistent( State(), - Event(f"foo{suffix}", relation=Relation("bar")), + _Event(f"foo{suffix}", relation=Relation("bar")), _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "xxx"}}}), ) assert_consistent( State(relations=[Relation("bar")]), - Event(f"bar{suffix}", relation=Relation("bar")), + _Event(f"bar{suffix}", relation=Relation("bar")), _CharmSpec(MyCharm, {"requires": {"bar": {"interface": "xxx"}}}), ) @pytest.mark.parametrize("suffix", RELATION_EVENTS_SUFFIX) def test_evt_no_relation(suffix): - assert_inconsistent(State(), Event(f"foo{suffix}"), _CharmSpec(MyCharm, {})) + assert_inconsistent(State(), _Event(f"foo{suffix}"), _CharmSpec(MyCharm, {})) assert_consistent( State(relations=[Relation("bar")]), - Event(f"bar{suffix}", relation=Relation("bar")), + _Event(f"bar{suffix}", relation=Relation("bar")), _CharmSpec(MyCharm, {"requires": {"bar": {"interface": "xxx"}}}), ) @@ -131,12 +131,12 @@ def test_evt_no_relation(suffix): def test_config_key_missing_from_meta(): assert_inconsistent( State(config={"foo": True}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}), ) assert_consistent( State(config={"foo": True}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "boolean"}}}), ) @@ -144,17 +144,17 @@ def test_config_key_missing_from_meta(): def test_bad_config_option_type(): assert_inconsistent( State(config={"foo": True}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "string"}}}), ) assert_inconsistent( State(config={"foo": True}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {}}}), ) assert_consistent( State(config={"foo": True}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "boolean"}}}), ) @@ -172,12 +172,12 @@ def test_config_types(config_type): type_name, valid_value, invalid_value = config_type assert_consistent( State(config={"foo": valid_value}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": type_name}}}), ) assert_inconsistent( State(config={"foo": invalid_value}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": type_name}}}), ) @@ -186,28 +186,28 @@ def test_config_types(config_type): def test_config_secret(juju_version): assert_consistent( State(config={"foo": "secret:co28kefmp25c77utl3n0"}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}), juju_version=juju_version, ) assert_inconsistent( State(config={"foo": 1}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}), ) assert_inconsistent( State(config={"foo": "co28kefmp25c77utl3n0"}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}), ) assert_inconsistent( State(config={"foo": "secret:secret"}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}), ) assert_inconsistent( State(config={"foo": "secret:co28kefmp25c77utl3n!"}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}), ) @@ -216,7 +216,7 @@ def test_config_secret(juju_version): def test_config_secret_old_juju(juju_version): assert_inconsistent( State(config={"foo": "secret:co28kefmp25c77utl3n0"}), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}), juju_version=juju_version, ) @@ -227,7 +227,7 @@ def test_secrets_jujuv_bad(bad_v): secret = Secret("secret:foo", {0: {"a": "b"}}) assert_inconsistent( State(secrets=[secret]), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}), bad_v, ) @@ -250,7 +250,7 @@ def test_secrets_jujuv_bad(bad_v): def test_secrets_jujuv_bad(good_v): assert_consistent( State(secrets=[Secret("secret:foo", {0: {"a": "b"}})]), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {}), good_v, ) @@ -259,12 +259,12 @@ def test_secrets_jujuv_bad(good_v): def test_peer_relation_consistency(): assert_inconsistent( State(relations=[Relation("foo")]), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {"peers": {"foo": {"interface": "bar"}}}), ) assert_consistent( State(relations=[PeerRelation("foo")]), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {"peers": {"foo": {"interface": "bar"}}}), ) @@ -272,7 +272,7 @@ def test_peer_relation_consistency(): def test_duplicate_endpoints_inconsistent(): assert_inconsistent( State(), - Event("bar"), + _Event("bar"), _CharmSpec( MyCharm, { @@ -286,7 +286,7 @@ def test_duplicate_endpoints_inconsistent(): def test_sub_relation_consistency(): assert_inconsistent( State(relations=[Relation("foo")]), - Event("bar"), + _Event("bar"), _CharmSpec( MyCharm, {"requires": {"foo": {"interface": "bar", "scope": "container"}}}, @@ -295,7 +295,7 @@ def test_sub_relation_consistency(): assert_consistent( State(relations=[SubordinateRelation("foo")]), - Event("bar"), + _Event("bar"), _CharmSpec( MyCharm, {"requires": {"foo": {"interface": "bar", "scope": "container"}}}, @@ -306,7 +306,7 @@ def test_sub_relation_consistency(): def test_relation_sub_inconsistent(): assert_inconsistent( State(relations=[SubordinateRelation("foo")]), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), ) @@ -314,7 +314,7 @@ def test_relation_sub_inconsistent(): def test_dupe_containers_inconsistent(): assert_inconsistent( State(containers=[Container("foo"), Container("foo")]), - Event("bar"), + _Event("bar"), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) @@ -366,7 +366,7 @@ def test_action_name(): ) assert_inconsistent( State(), - Event("box_action", action=action), + _Event("box_action", action=action), _CharmSpec(MyCharm, meta={}, actions={"foo": {}}), ) @@ -405,7 +405,7 @@ def test_action_params_type(ptype, good, bad): def test_duplicate_relation_ids(): assert_inconsistent( State(relations=[Relation("foo", id=1), Relation("bar", id=1)]), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -418,13 +418,13 @@ def test_duplicate_relation_ids(): def test_relation_without_endpoint(): assert_inconsistent( State(relations=[Relation("foo", id=1), Relation("bar", id=1)]), - Event("start"), + _Event("start"), _CharmSpec(MyCharm, meta={"name": "charlemagne"}), ) assert_consistent( State(relations=[Relation("foo", id=1), Relation("bar", id=2)]), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -438,12 +438,12 @@ def test_storage_event(): storage = Storage("foo") assert_inconsistent( State(storage=[storage]), - Event("foo-storage-attached"), + _Event("foo-storage-attached"), _CharmSpec(MyCharm, meta={"name": "rupert"}), ) assert_inconsistent( State(storage=[storage]), - Event("foo-storage-attached"), + _Event("foo-storage-attached"), _CharmSpec( MyCharm, meta={"name": "rupert", "storage": {"foo": {"type": "filesystem"}}} ), @@ -463,19 +463,19 @@ def test_storage_states(): assert_inconsistent( State(storage=[storage1, storage2]), - Event("start"), + _Event("start"), _CharmSpec(MyCharm, meta={"name": "everett"}), ) assert_consistent( State(storage=[storage1, dataclasses.replace(storage2, index=2)]), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "frank", "storage": {"foo": {"type": "filesystem"}}} ), ) assert_consistent( State(storage=[storage1, dataclasses.replace(storage2, name="marx")]), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -493,7 +493,7 @@ def test_resource_states(): # happy path assert_consistent( State(resources={"foo": "/foo/bar.yaml"}), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "yamlman", "resources": {"foo": {"type": "oci-image"}}}, @@ -503,7 +503,7 @@ def test_resource_states(): # no resources in state but some in meta: OK. Not realistic wrt juju but fine for testing assert_consistent( State(), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "yamlman", "resources": {"foo": {"type": "oci-image"}}}, @@ -513,7 +513,7 @@ def test_resource_states(): # resource not defined in meta assert_inconsistent( State(resources={"bar": "/foo/bar.yaml"}), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "yamlman", "resources": {"foo": {"type": "oci-image"}}}, @@ -522,7 +522,7 @@ def test_resource_states(): assert_inconsistent( State(resources={"bar": "/foo/bar.yaml"}), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "yamlman"}, @@ -533,7 +533,7 @@ def test_resource_states(): def test_networks_consistency(): assert_inconsistent( State(networks={"foo": Network.default()}), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={"name": "wonky"}, @@ -542,7 +542,7 @@ def test_networks_consistency(): assert_inconsistent( State(networks={"foo": Network.default()}), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -555,7 +555,7 @@ def test_networks_consistency(): assert_consistent( State(networks={"foo": Network.default()}), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -577,7 +577,7 @@ def test_storedstate_consistency(): StoredState("OtherCharmLib", content={"foo": (1, 2, 3)}), ] ), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -592,7 +592,7 @@ def test_storedstate_consistency(): StoredState(None, "_stored", content={"foo": "bar"}), ] ), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ @@ -602,7 +602,7 @@ def test_storedstate_consistency(): ) assert_inconsistent( State(stored_state=[StoredState(None, content={"secret": Secret("foo", {})})]), - Event("start"), + _Event("start"), _CharmSpec( MyCharm, meta={ diff --git a/tests/test_context.py b/tests/test_context.py index 7d2b795c..d6995efc 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -3,8 +3,8 @@ import pytest from ops import CharmBase -from scenario import Action, Context, Event, State -from scenario.state import next_action_id +from scenario import Action, Context, State +from scenario.state import _Event, next_action_id class MyCharm(CharmBase): @@ -17,14 +17,14 @@ def test_run(): with patch.object(ctx, "_run") as p: ctx._output_state = "foo" # would normally be set within the _run call scope - output = ctx.run("start", state) + output = ctx.run(ctx.on.start(), state) assert output == "foo" assert p.called e = p.call_args.kwargs["event"] s = p.call_args.kwargs["state"] - assert isinstance(e, Event) + assert isinstance(e, _Event) assert e.name == "start" assert s is state @@ -38,7 +38,8 @@ def test_run_action(): ctx._output_state = ( "foo" # would normally be set within the _run_action call scope ) - output = ctx.run_action("do-foo", state) + action = Action("do-foo") + output = ctx.run_action(action, state) assert output.state == "foo" assert p.called @@ -54,8 +55,7 @@ def test_run_action(): @pytest.mark.parametrize("app_name", ("foo", "bar", "george")) @pytest.mark.parametrize("unit_id", (1, 2, 42)) def test_app_name(app_name, unit_id): - with Context( - MyCharm, meta={"name": "foo"}, app_name=app_name, unit_id=unit_id - ).manager("start", State()) as mgr: + ctx = Context(MyCharm, meta={"name": "foo"}, app_name=app_name, unit_id=unit_id) + with ctx.manager(ctx.on.start(), State()) as mgr: assert mgr.charm.app.name == app_name assert mgr.charm.unit.name == f"{app_name}/{unit_id}" diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 1ba1e70a..e2d91a82 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -134,12 +134,10 @@ def test_revision_secret_events_as_positional_arg(event_name): "secret:123", {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, owner=None ) state_in = scenario.State(secrets=[secret]) - with pytest.assertRaises(ValueError): + with pytest.raises(TypeError): ctx.run(getattr(ctx.on, event_name)(secret, 42), state_in) -@pytest.mark.parametrize("as_kwarg", [True, False]) -@pytest.mark.parametrize("storage_index", [0, 1, None]) @pytest.mark.parametrize( "event_name, event_kind", [ @@ -147,43 +145,20 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(as_kwarg, storage_index, event_name, event_kind): +def test_storage_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storages = [ - scenario.Storage("foo", index) for index in range((storage_index or 0) + 1) - ] - state_in = scenario.State(storage=storages) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) # These look like: # ctx.run(ctx.on.storage_attached(storage), state) - if as_kwarg: - args = () - if storage_index is None: - kwargs = {"storage": storages[-1]} - else: - kwargs = {"storage": storages[-1], "index": storages[-1].index} - else: - args = (storages[-1],) - if storage_index is None: - kwargs = {} - else: - kwargs = {"index": storages[-1].index} - with ctx.manager(getattr(ctx.on, event_name)(*args, **kwargs), state_in) as mgr: + with ctx.manager(getattr(ctx.on, event_name)(storage), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storages[-1].name - assert event.storage.index == storages[-1].index - - -@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) -def test_storage_events_as_positional_arg(event_name): - ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storage = scenario.Storage("foo") - state_in = scenario.State(storage=[storage]) - with pytest.assertRaises(ValueError): - ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) + assert event.storage.name == storage.name + assert event.storage.index == storage.index def test_action_event_no_params(): @@ -191,7 +166,7 @@ def test_action_event_no_params(): # These look like: # ctx.run_action(ctx.on.action(action), state) action = scenario.Action("act") - with ctx.action_manager(ctx.on.action(action), scenario.State()) as mgr: + with ctx.action_manager(action, scenario.State()) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -203,9 +178,9 @@ def test_action_event_with_params(): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) action = scenario.Action("act", {"param": "hello"}) # These look like: - # ctx.run_action(ctx.on.act_action(action=action), state) + # ctx.run_action(ctx.on.action(action=action), state) # So that any parameters can be included and the ID can be customised. - with ctx.action_manager(ctx.on.act_action(action=action), scenario.State()) as mgr: + with ctx.action_manager(action, scenario.State()) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) @@ -256,7 +231,7 @@ def test_relation_app_events(as_kwarg, event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.relation.id == relation.relation_id + assert event.relation.id == relation.id assert event.app.name == relation.remote_app_name assert event.unit is None @@ -266,7 +241,7 @@ def test_relation_events_as_positional_arg(event_name): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) relation = scenario.Relation("baz") state_in = scenario.State(relations=[relation]) - with pytest.assertRaises(ValueError): + with pytest.raises(TypeError): ctx.run(getattr(ctx.on, event_name)(relation, 0), state_in) @@ -290,7 +265,7 @@ def test_relation_unit_events_default_unit(event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.relation.id == relation.relation_id + assert event.relation.id == relation.id assert event.app.name == relation.remote_app_name assert event.unit.name == "remote/1" @@ -310,13 +285,15 @@ def test_relation_unit_events(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) - with ctx.manager(getattr(ctx.on, event_name)(relation, unit=2), state_in) as mgr: + with ctx.manager( + getattr(ctx.on, event_name)(relation, remote_unit=2), state_in + ) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.relation.id == relation.relation_id + assert event.relation.id == relation.id assert event.app.name == relation.remote_app_name assert event.unit.name == "remote/2" @@ -328,14 +305,14 @@ def test_relation_departed_event(): # These look like: # ctx.run(ctx.on.baz_relation_departed(unit=unit_ordinal, departing_unit=unit_ordinal), state) with ctx.manager( - ctx.on.relation_departed(relation, unit=2, departing_unit=1), state_in + ctx.on.relation_departed(relation, remote_unit=2, departing_unit=1), state_in ) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, ops.RelationDepartedEvent) - assert event.relation.id == relation.relation_id + assert event.relation.id == relation.id assert event.app.name == relation.remote_app_name assert event.unit.name == "remote/2" assert event.departing_unit.name == "remote/1" diff --git a/tests/test_e2e/test_actions.py b/tests/test_e2e/test_actions.py index 30cc9d1e..6256885c 100644 --- a/tests/test_e2e/test_actions.py +++ b/tests/test_e2e/test_actions.py @@ -5,8 +5,7 @@ from scenario import Context from scenario.context import InvalidEventError -from scenario.state import Action, Event, State -from tests.helpers import trigger +from scenario.state import Action, State, _Event @pytest.fixture(scope="function") @@ -65,13 +64,6 @@ def test_cannot_run_action(mycharm): ctx.run(action, state=State()) -def test_cannot_run_action_name(mycharm): - ctx = Context(mycharm, meta={"name": "foo"}, actions={"foo": {}}) - action = Action("foo") - with pytest.raises(InvalidEventError): - ctx.run(action.event.name, state=State()) - - def test_cannot_run_action_event(mycharm): ctx = Context(mycharm, meta={"name": "foo"}, actions={"foo": {}}) action = Action("foo") diff --git a/tests/test_e2e/test_config.py b/tests/test_e2e/test_config.py index 27b25c29..01737ec8 100644 --- a/tests/test_e2e/test_config.py +++ b/tests/test_e2e/test_config.py @@ -2,7 +2,7 @@ from ops.charm import CharmBase from ops.framework import Framework -from scenario.state import Event, Network, Relation, State, _CharmSpec +from scenario.state import Network, Relation, State, _CharmSpec, _Event from tests.helpers import trigger diff --git a/tests/test_e2e/test_custom_event_triggers.py b/tests/test_e2e/test_custom_event_triggers.py deleted file mode 100644 index 1c6f07cd..00000000 --- a/tests/test_e2e/test_custom_event_triggers.py +++ /dev/null @@ -1,146 +0,0 @@ -import os -from unittest.mock import MagicMock - -import pytest -from ops.charm import CharmBase, CharmEvents -from ops.framework import EventBase, EventSource, Object - -from scenario import State -from scenario.ops_main_mock import NoObserverError -from scenario.runtime import InconsistentScenarioError -from tests.helpers import trigger - - -def test_custom_event_emitted(): - class FooEvent(EventBase): - pass - - class MyCharmEvents(CharmEvents): - foo = EventSource(FooEvent) - - class MyCharm(CharmBase): - META = {"name": "mycharm"} - on = MyCharmEvents() - _foo_called = 0 - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.framework.observe(self.on.foo, self._on_foo) - self.framework.observe(self.on.start, self._on_start) - - def _on_foo(self, e): - MyCharm._foo_called += 1 - - def _on_start(self, e): - self.on.foo.emit() - - trigger(State(), "foo", MyCharm, meta=MyCharm.META) - assert MyCharm._foo_called == 1 - - trigger(State(), "start", MyCharm, meta=MyCharm.META) - assert MyCharm._foo_called == 2 - - -def test_funky_named_event_emitted(): - class FooRelationChangedEvent(EventBase): - pass - - class MyCharmEvents(CharmEvents): - foo_relation_changed = EventSource(FooRelationChangedEvent) - - class MyCharm(CharmBase): - META = {"name": "mycharm"} - on = MyCharmEvents() - _foo_called = False - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.framework.observe(self.on.foo_relation_changed, self._on_foo) - - def _on_foo(self, e): - MyCharm._foo_called = True - - # we called our custom event like a builtin one. Trouble! - with pytest.raises(InconsistentScenarioError): - trigger(State(), "foo-relation-changed", MyCharm, meta=MyCharm.META) - - assert not MyCharm._foo_called - - os.environ["SCENARIO_SKIP_CONSISTENCY_CHECKS"] = "1" - trigger(State(), "foo-relation-changed", MyCharm, meta=MyCharm.META) - assert MyCharm._foo_called - os.unsetenv("SCENARIO_SKIP_CONSISTENCY_CHECKS") - - -def test_child_object_event_emitted_no_path_raises(): - class FooEvent(EventBase): - pass - - class MyObjEvents(CharmEvents): - foo = EventSource(FooEvent) - - class MyObject(Object): - my_on = MyObjEvents() - - class MyCharm(CharmBase): - META = {"name": "mycharm"} - _foo_called = False - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.obj = MyObject(self, "child") - self.framework.observe(self.obj.my_on.foo, self._on_foo) - - def _on_foo(self, e): - MyCharm._foo_called = True - - with pytest.raises(NoObserverError): - # this will fail because "foo" isn't registered on MyCharm but on MyCharm.foo - trigger(State(), "foo", MyCharm, meta=MyCharm.META) - assert MyCharm._foo_called - - # workaround: we can use pre_event to have Scenario set up the simulation for us and run our - # test code before it eventually fails. pre_event gets called with the set-up charm instance. - def pre_event(charm: MyCharm): - event_mock = MagicMock() - charm._on_foo(event_mock) - assert charm.unit.name == "mycharm/0" - - # make sure you only filter out NoObserverError, else if pre_event raises, - # they will also be caught while you want them to bubble up. - with pytest.raises(NoObserverError): - trigger( - State(), - "rubbish", # you can literally put anything here - MyCharm, - pre_event=pre_event, - meta=MyCharm.META, - ) - assert MyCharm._foo_called - - -def test_child_object_event(): - class FooEvent(EventBase): - pass - - class MyObjEvents(CharmEvents): - foo = EventSource(FooEvent) - - class MyObject(Object): - my_on = MyObjEvents() - - class MyCharm(CharmBase): - META = {"name": "mycharm"} - _foo_called = False - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.obj = MyObject(self, "child") - self.framework.observe(self.obj.my_on.foo, self._on_foo) - - def _on_foo(self, e): - MyCharm._foo_called = True - - trigger(State(), "obj.my_on.foo", MyCharm, meta=MyCharm.META) - - assert MyCharm._foo_called diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index a87c89a7..c5394f14 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -131,13 +131,16 @@ def test_deferred_relation_event(mycharm): def test_deferred_relation_event_from_relation(mycharm): + ctx = Context(mycharm, meta=mycharm.META) mycharm.defer_next = 2 rel = Relation(endpoint="foo", remote_app_name="remote") out = trigger( State( relations=[rel], deferred=[ - rel.changed_event(remote_unit_id=1).deferred(handler=mycharm._on_event) + ctx.on.relation_changed(rel, remote_unit=1).deferred( + handler=mycharm._on_event + ) ], ), "start", diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index 40da6bce..fde6badc 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -3,7 +3,7 @@ from ops import CharmBase, StartEvent, UpdateStatusEvent from scenario import Context -from scenario.state import Event, State, _CharmSpec, _EventType +from scenario.state import State, _CharmSpec, _Event, _EventType @pytest.mark.parametrize( @@ -27,7 +27,7 @@ ), ) def test_event_type(evt, expected_type): - event = Event(evt) + event = _Event(evt) assert event._path.type is expected_type assert event._is_relation_event is (expected_type is _EventType.relation) @@ -85,7 +85,7 @@ def _foo(self, e): capture_framework_events=True, ) ctx.run( - ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)]) + ctx.on.start(), State(deferred=[_Event("update-status").deferred(MyCharm._foo)]) ) assert len(ctx.emitted_events) == 5 diff --git a/tests/test_e2e/test_event_bind.py b/tests/test_e2e/test_event_bind.py index 4878e6ac..c0d9abfb 100644 --- a/tests/test_e2e/test_event_bind.py +++ b/tests/test_e2e/test_event_bind.py @@ -1,32 +1,31 @@ import pytest -from scenario import Container, Event, Relation, Secret, State -from scenario.state import BindFailedError +from scenario import Container, Relation, Secret, State def test_bind_relation(): - event = Event("foo-relation-changed") + event = _Event("foo-relation-changed") foo_relation = Relation("foo") state = State(relations=[foo_relation]) assert event.bind(state).relation is foo_relation def test_bind_relation_complex_name(): - event = Event("foo-bar-baz-relation-changed") + event = _Event("foo-bar-baz-relation-changed") foo_relation = Relation("foo_bar_baz") state = State(relations=[foo_relation]) assert event.bind(state).relation is foo_relation def test_bind_relation_notfound(): - event = Event("foo-relation-changed") + event = _Event("foo-relation-changed") state = State(relations=[]) with pytest.raises(BindFailedError): event.bind(state) def test_bind_relation_toomany(caplog): - event = Event("foo-relation-changed") + event = _Event("foo-relation-changed") foo_relation = Relation("foo") foo_relation1 = Relation("foo") state = State(relations=[foo_relation, foo_relation1]) @@ -35,28 +34,28 @@ def test_bind_relation_toomany(caplog): def test_bind_secret(): - event = Event("secret-changed") + event = _Event("secret-changed") secret = Secret("foo", {"a": "b"}) state = State(secrets=[secret]) assert event.bind(state).secret is secret def test_bind_secret_notfound(): - event = Event("secret-changed") + event = _Event("secret-changed") state = State(secrets=[]) with pytest.raises(BindFailedError): event.bind(state) def test_bind_container(): - event = Event("foo-pebble-ready") + event = _Event("foo-pebble-ready") container = Container("foo") state = State(containers=[container]) assert event.bind(state).container is container def test_bind_container_notfound(): - event = Event("foo-pebble-ready") + event = _Event("foo-pebble-ready") state = State(containers=[]) with pytest.raises(BindFailedError): event.bind(state) diff --git a/tests/test_e2e/test_juju_log.py b/tests/test_e2e/test_juju_log.py index e76c04bc..1efd1e70 100644 --- a/tests/test_e2e/test_juju_log.py +++ b/tests/test_e2e/test_juju_log.py @@ -5,7 +5,6 @@ from scenario import Context from scenario.state import JujuLogLine, State -from tests.helpers import trigger logger = logging.getLogger("testing logger") diff --git a/tests/test_e2e/test_manager.py b/tests/test_e2e/test_manager.py index 28cbe516..66d39f82 100644 --- a/tests/test_e2e/test_manager.py +++ b/tests/test_e2e/test_manager.py @@ -31,7 +31,7 @@ def _on_event(self, e): def test_manager(mycharm): ctx = Context(mycharm, meta=mycharm.META) - with _EventManager(ctx, "start", State()) as manager: + with _EventManager(ctx, ctx.on.start(), State()) as manager: assert isinstance(manager.charm, mycharm) assert not manager.output state_out = manager.run() @@ -43,7 +43,7 @@ def test_manager(mycharm): def test_manager_implicit(mycharm): ctx = Context(mycharm, meta=mycharm.META) - with _EventManager(ctx, "start", State()) as manager: + with _EventManager(ctx, ctx.on.start(), State()) as manager: assert isinstance(manager.charm, mycharm) # do not call .run() @@ -55,7 +55,7 @@ def test_manager_implicit(mycharm): def test_manager_reemit_fails(mycharm): ctx = Context(mycharm, meta=mycharm.META) - with _EventManager(ctx, "start", State()) as manager: + with _EventManager(ctx, ctx.on.start(), State()) as manager: manager.run() with pytest.raises(AlreadyEmittedError): manager.run() @@ -65,7 +65,7 @@ def test_manager_reemit_fails(mycharm): def test_context_manager(mycharm): ctx = Context(mycharm, meta=mycharm.META) - with ctx.manager("start", State()) as manager: + with ctx.manager(ctx.on.start(), State()) as manager: state_out = manager.run() assert isinstance(state_out, State) assert ctx.emitted_events[0].handle.kind == "start" diff --git a/tests/test_e2e/test_network.py b/tests/test_e2e/test_network.py index c3d271df..440a01be 100644 --- a/tests/test_e2e/test_network.py +++ b/tests/test_e2e/test_network.py @@ -41,7 +41,7 @@ def test_ip_get(mycharm): ) with ctx.manager( - "update_status", + ctx.on.update_status(), State( relations=[ Relation( @@ -78,7 +78,7 @@ def test_no_sub_binding(mycharm): ) with ctx.manager( - "update_status", + ctx.on.update_status(), State( relations=[ SubordinateRelation("bar"), @@ -103,7 +103,7 @@ def test_no_relation_error(mycharm): ) with ctx.manager( - "update_status", + ctx.on.update_status(), State( relations=[ Relation( diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 6352b3a4..cca442dd 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -129,7 +129,7 @@ def callback(self: CharmBase): charm_type=charm_cls, meta={"name": "foo", "containers": {"foo": {}}}, ) - with ctx.manager("start", state=state) as mgr: + with ctx.manager(ctx.on.start(), state=state) as mgr: out = mgr.run() callback(mgr.charm) @@ -317,9 +317,8 @@ def test_exec_wait_error(charm_cls): ] ) - with Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}).manager( - "start", state - ) as mgr: + ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) + with ctx.manager(ctx.on.start(), state) as mgr: container = mgr.charm.unit.get_container("foo") proc = container.exec(["foo"]) with pytest.raises(ExecError): @@ -340,9 +339,8 @@ def test_exec_wait_output(charm_cls): ] ) - with Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}).manager( - "start", state - ) as mgr: + ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) + with ctx.manager(ctx.on.start(), state) as mgr: container = mgr.charm.unit.get_container("foo") proc = container.exec(["foo"]) out, err = proc.wait_output() @@ -361,9 +359,8 @@ def test_exec_wait_output_error(charm_cls): ] ) - with Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}).manager( - "start", state - ) as mgr: + ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) + with ctx.manager(ctx.on.start(), state) as mgr: container = mgr.charm.unit.get_container("foo") proc = container.exec(["foo"]) with pytest.raises(ExecError): diff --git a/tests/test_e2e/test_ports.py b/tests/test_e2e/test_ports.py index 8343c976..13502971 100644 --- a/tests/test_e2e/test_ports.py +++ b/tests/test_e2e/test_ports.py @@ -27,7 +27,7 @@ def ctx(): def test_open_port(ctx): - out = ctx.run(ctx.start(), State()) + out = ctx.run(ctx.on.start(), State()) port = out.opened_ports.pop() assert port.protocol == "tcp" diff --git a/tests/test_e2e/test_relations.py b/tests/test_e2e/test_relations.py index f0ef1058..bd2e9751 100644 --- a/tests/test_e2e/test_relations.py +++ b/tests/test_e2e/test_relations.py @@ -82,6 +82,7 @@ def pre_event(charm: CharmBase): }, }, config={"options": {"foo": {"type": "string"}}}, + pre_event=pre_event, ) @@ -99,7 +100,7 @@ def test_relation_events(mycharm, evt_name): relation, ], ), - getattr(relation, f"{evt_name}_event"), + f"relation_{evt_name}", mycharm, meta={ "name": "local", @@ -143,7 +144,7 @@ def callback(charm: CharmBase, e): relation, ], ), - getattr(relation, f"{evt_name}_event"), + f"relation_{evt_name}", mycharm, meta={ "name": "local", @@ -182,14 +183,8 @@ def callback(charm: CharmBase, event): mycharm._call = callback - trigger( - State( - relations=[ - relation, - ], - ), - getattr(relation, f"{evt_name}_event")(remote_unit_id=remote_unit_id), - mycharm, + ctx = Context( + charm_type=mycharm, meta={ "name": "local", "requires": { @@ -197,6 +192,11 @@ def callback(charm: CharmBase, event): }, }, ) + state = State(relations=[relation]) + event = getattr(ctx.on, f"relation_{evt_name}")( + relation, remote_unit=remote_unit_id + ) + ctx.run(event, state=state) @pytest.mark.parametrize( @@ -235,7 +235,7 @@ def callback(charm: CharmBase, event): relation, ], ), - getattr(relation, f"{evt_name}_event"), + f"relation_{evt_name}", mycharm, meta={ "name": "local", @@ -295,7 +295,7 @@ def callback(charm: CharmBase, event): relation, ], ), - getattr(relation, f"{evt_name}_event"), + f"relation_{evt_name}", mycharm, meta={ "name": "local", @@ -346,7 +346,7 @@ def test_relation_event_trigger(relation, evt_name, mycharm): } state = trigger( State(relations=[relation]), - getattr(relation, evt_name + "_event"), + f"relation_{evt_name}", mycharm, meta=meta, ) @@ -379,7 +379,7 @@ def post_event(charm: CharmBase): trigger( State(relations=[sub1, sub2]), - "update-status", + "update_status", mycharm, meta=meta, post_event=post_event, @@ -403,9 +403,10 @@ def test_relation_ids(): def test_broken_relation_not_in_model_relations(mycharm): rel = Relation("foo") - with Context( + ctx = Context( mycharm, meta={"name": "local", "requires": {"foo": {"interface": "foo"}}} - ).manager(rel.broken_event, state=State(relations=[rel])) as mgr: + ) + with ctx.manager(ctx.on.relation_broken(rel), state=State(relations=[rel])) as mgr: charm = mgr.charm assert charm.model.get_relation("foo") is None diff --git a/tests/test_e2e/test_rubbish_events.py b/tests/test_e2e/test_rubbish_events.py index 10582d82..b20ae4a9 100644 --- a/tests/test_e2e/test_rubbish_events.py +++ b/tests/test_e2e/test_rubbish_events.py @@ -5,7 +5,7 @@ from ops.framework import EventBase, EventSource, Framework, Object from scenario.ops_main_mock import NoObserverError -from scenario.state import Container, Event, State, _CharmSpec +from scenario.state import Container, State, _CharmSpec, _Event from tests.helpers import trigger @@ -86,4 +86,4 @@ def test_is_custom_event(mycharm, evt_name, expected): spec = _CharmSpec( charm_type=mycharm, meta={"name": "mycharm", "requires": {"foo": {}}} ) - assert Event(evt_name)._is_builtin_event(spec) is expected + assert _Event(evt_name)._is_builtin_event(spec) is expected diff --git a/tests/test_e2e/test_secrets.py b/tests/test_e2e/test_secrets.py index dfb7bb11..d0e6dabb 100644 --- a/tests/test_e2e/test_secrets.py +++ b/tests/test_e2e/test_secrets.py @@ -28,9 +28,8 @@ def _on_event(self, event): def test_get_secret_no_secret(mycharm): - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", State() - ) as mgr: + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager(ctx.on.update_status(), State()) as mgr: with pytest.raises(SecretNotFoundError): assert mgr.charm.model.get_secret(id="foo") with pytest.raises(SecretNotFoundError): @@ -38,17 +37,19 @@ def test_get_secret_no_secret(mycharm): def test_get_secret(mycharm): - with Context(mycharm, meta={"name": "local"}).manager( + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( state=State(secrets=[Secret(id="foo", contents={0: {"a": "b"}})]), - event="update_status", + event=ctx.on.update_status(), ) as mgr: assert mgr.charm.model.get_secret(id="foo").get_content()["a"] == "b" @pytest.mark.parametrize("owner", ("app", "unit")) def test_get_secret_get_refresh(mycharm, owner): - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State( secrets=[ Secret( @@ -68,8 +69,9 @@ def test_get_secret_get_refresh(mycharm, owner): @pytest.mark.parametrize("app", (True, False)) def test_get_secret_nonowner_peek_update(mycharm, app): - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State( leader=app, secrets=[ @@ -94,8 +96,9 @@ def test_get_secret_nonowner_peek_update(mycharm, app): @pytest.mark.parametrize("owner", ("app", "unit")) def test_get_secret_owner_peek_update(mycharm, owner): - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State( secrets=[ Secret( @@ -145,8 +148,9 @@ def test_consumer_events_failures(mycharm, evt_prefix): @pytest.mark.parametrize("app", (True, False)) def test_add(mycharm, app): - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State(leader=app), ) as mgr: charm = mgr.charm @@ -165,8 +169,9 @@ def test_set_legacy_behaviour(mycharm): # in juju < 3.1.7, secret owners always used to track the latest revision. # ref: https://bugs.launchpad.net/juju/+bug/2037120 rev1, rev2, rev3 = {"foo": "bar"}, {"foo": "baz"}, {"foo": "baz", "qux": "roz"} - with Context(mycharm, meta={"name": "local"}, juju_version="3.1.6").manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}, juju_version="3.1.6") + with ctx.manager( + ctx.on.update_status(), State(), ) as mgr: charm = mgr.charm @@ -204,8 +209,9 @@ def test_set_legacy_behaviour(mycharm): def test_set(mycharm): rev1, rev2, rev3 = {"foo": "bar"}, {"foo": "baz"}, {"foo": "baz", "qux": "roz"} - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State(), ) as mgr: charm = mgr.charm @@ -235,8 +241,9 @@ def test_set(mycharm): def test_set_juju33(mycharm): rev1, rev2, rev3 = {"foo": "bar"}, {"foo": "baz"}, {"foo": "baz", "qux": "roz"} - with Context(mycharm, meta={"name": "local"}, juju_version="3.3.1").manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}, juju_version="3.3.1") + with ctx.manager( + ctx.on.update_status(), State(), ) as mgr: charm = mgr.charm @@ -263,8 +270,9 @@ def test_set_juju33(mycharm): @pytest.mark.parametrize("app", (True, False)) def test_meta(mycharm, app): - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State( leader=True, secrets=[ @@ -302,8 +310,9 @@ def test_secret_permission_model(mycharm, leader, owner): or (owner == "unit") ) - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State( leader=leader, secrets=[ @@ -352,10 +361,11 @@ def test_secret_permission_model(mycharm, leader, owner): @pytest.mark.parametrize("app", (True, False)) def test_grant(mycharm, app): - with Context( + ctx = Context( mycharm, meta={"name": "local", "requires": {"foo": {"interface": "bar"}}} - ).manager( - "update_status", + ) + with ctx.manager( + ctx.on.update_status(), State( relations=[Relation("foo", "remote")], secrets=[ @@ -386,8 +396,9 @@ def test_grant(mycharm, app): def test_update_metadata(mycharm): exp = datetime.datetime(2050, 12, 12) - with Context(mycharm, meta={"name": "local"}).manager( - "update_status", + ctx = Context(mycharm, meta={"name": "local"}) + with ctx.manager( + ctx.on.update_status(), State( secrets=[ Secret( @@ -474,7 +485,7 @@ class GrantingCharm(CharmBase): def __init__(self, *args): super().__init__(*args) - context = Context( + ctx = Context( GrantingCharm, meta={"name": "foo", "provides": {"bar": {"interface": "bar"}}} ) relation_remote_app = "remote_secret_desirerer" @@ -487,7 +498,7 @@ def __init__(self, *args): ], ) - with context.manager("start", state) as mgr: + with ctx.manager(ctx.on.start(), state) as mgr: charm = mgr.charm secret = charm.app.add_secret({"foo": "bar"}, label="mylabel") bar_relation = charm.model.relations["bar"][0] @@ -498,7 +509,7 @@ def __init__(self, *args): scenario_secret = mgr.output.secrets[0] assert relation_remote_app in scenario_secret.remote_grants[relation_id] - with context.manager("start", mgr.output) as mgr: + with ctx.manager(ctx.on.start(), mgr.output) as mgr: charm: GrantingCharm = mgr.charm secret = charm.model.get_secret(label="mylabel") secret.revoke(bar_relation) @@ -506,7 +517,7 @@ def __init__(self, *args): scenario_secret = mgr.output.secrets[0] assert scenario_secret.remote_grants == {} - with context.manager("start", mgr.output) as mgr: + with ctx.manager(ctx.on.start(), mgr.output) as mgr: charm: GrantingCharm = mgr.charm secret = charm.model.get_secret(label="mylabel") secret.remove_all_revisions() diff --git a/tests/test_e2e/test_status.py b/tests/test_e2e/test_status.py index 43231597..6b3bc0c5 100644 --- a/tests/test_e2e/test_status.py +++ b/tests/test_e2e/test_status.py @@ -131,7 +131,7 @@ def _on_update_status(self, _): meta={"name": "local"}, ) - out = ctx.run(ctx.ȯn.install(), State(leader=True)) + out = ctx.run(ctx.on.install(), State(leader=True)) out = ctx.run(ctx.on.start(), out) out = ctx.run(ctx.on.update_status(), out) diff --git a/tests/test_e2e/test_storage.py b/tests/test_e2e/test_storage.py index ec9fa0fb..b62288bb 100644 --- a/tests/test_e2e/test_storage.py +++ b/tests/test_e2e/test_storage.py @@ -23,13 +23,13 @@ def no_storage_ctx(): def test_storage_get_null(no_storage_ctx): - with no_storage_ctx.manager("update-status", State()) as mgr: + with no_storage_ctx.manager(no_storage_ctx.on.update_status(), State()) as mgr: storages = mgr.charm.model.storages assert not len(storages) def test_storage_get_unknown_name(storage_ctx): - with storage_ctx.manager("update-status", State()) as mgr: + with storage_ctx.manager(storage_ctx.on.update_status(), State()) as mgr: storages = mgr.charm.model.storages # not in metadata with pytest.raises(KeyError): @@ -37,7 +37,7 @@ def test_storage_get_unknown_name(storage_ctx): def test_storage_request_unknown_name(storage_ctx): - with storage_ctx.manager("update-status", State()) as mgr: + with storage_ctx.manager(storage_ctx.on.update_status(), State()) as mgr: storages = mgr.charm.model.storages # not in metadata with pytest.raises(ModelError): @@ -45,7 +45,7 @@ def test_storage_request_unknown_name(storage_ctx): def test_storage_get_some(storage_ctx): - with storage_ctx.manager("update-status", State()) as mgr: + with storage_ctx.manager(storage_ctx.on.update_status(), State()) as mgr: storages = mgr.charm.model.storages # known but none attached assert storages["foo"] == [] @@ -53,7 +53,7 @@ def test_storage_get_some(storage_ctx): @pytest.mark.parametrize("n", (1, 3, 5)) def test_storage_add(storage_ctx, n): - with storage_ctx.manager("update-status", State()) as mgr: + with storage_ctx.manager(storage_ctx.on.update_status(), State()) as mgr: storages = mgr.charm.model.storages storages.request("foo", n) @@ -65,7 +65,9 @@ def test_storage_usage(storage_ctx): # setup storage with some content (storage.get_filesystem(storage_ctx) / "myfile.txt").write_text("helloworld") - with storage_ctx.manager("update-status", State(storage=[storage])) as mgr: + with storage_ctx.manager( + storage_ctx.on.update_status(), State(storage=[storage]) + ) as mgr: foo = mgr.charm.model.storages["foo"][0] loc = foo.location path = loc / "myfile.txt" diff --git a/tests/test_e2e/test_vroot.py b/tests/test_e2e/test_vroot.py index 6b6f902e..c8702611 100644 --- a/tests/test_e2e/test_vroot.py +++ b/tests/test_e2e/test_vroot.py @@ -55,8 +55,9 @@ def test_charm_virtual_root_cleanup_if_exists(charm_virtual_root): raw_ori_meta = yaml.safe_dump({"name": "karl"}) meta_file.write_text(raw_ori_meta) - with Context(MyCharm, meta=MyCharm.META, charm_root=charm_virtual_root).manager( - "start", + ctx = Context(MyCharm, meta=MyCharm.META, charm_root=charm_virtual_root) + with ctx.manager( + ctx.on.start(), State(), ) as mgr: assert meta_file.exists() @@ -77,8 +78,9 @@ def test_charm_virtual_root_cleanup_if_not_exists(charm_virtual_root): assert not meta_file.exists() - with Context(MyCharm, meta=MyCharm.META, charm_root=charm_virtual_root).manager( - "start", + ctx = Context(MyCharm, meta=MyCharm.META, charm_root=charm_virtual_root) + with ctx.manager( + ctx.on.start(), State(), ) as mgr: assert meta_file.exists() diff --git a/tests/test_emitted_events_util.py b/tests/test_emitted_events_util.py index 7fc0eb00..e2cec7d2 100644 --- a/tests/test_emitted_events_util.py +++ b/tests/test_emitted_events_util.py @@ -2,8 +2,9 @@ from ops.charm import CharmBase, CharmEvents, CollectStatusEvent, StartEvent from ops.framework import CommitEvent, EventBase, EventSource, PreCommitEvent -from scenario import Event, State +from scenario import State from scenario.capture_events import capture_events +from scenario.state import _Event from tests.helpers import trigger @@ -71,7 +72,7 @@ def test_capture_deferred_evt(): # todo: this test should pass with ops < 2.1 as well with capture_events() as emitted: trigger( - State(deferred=[Event("foo").deferred(handler=MyCharm._on_foo)]), + State(deferred=[_Event("foo").deferred(handler=MyCharm._on_foo)]), "start", MyCharm, meta=MyCharm.META, @@ -87,7 +88,7 @@ def test_capture_no_deferred_evt(): # todo: this test should pass with ops < 2.1 as well with capture_events(include_deferred=False) as emitted: trigger( - State(deferred=[Event("foo").deferred(handler=MyCharm._on_foo)]), + State(deferred=[_Event("foo").deferred(handler=MyCharm._on_foo)]), "start", MyCharm, meta=MyCharm.META, diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 7145a344..38bca584 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -21,7 +21,7 @@ def context(): return Context(charm_type=MyCharm, meta={"name": "foo"}) def test_sth(context): - context.run(ctx.on.start(), State()) + context.run(context.on.start(), State()) """ ) diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 1fa0e884..eaaf99ef 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -10,7 +10,7 @@ from scenario import Context from scenario.runtime import Runtime, UncaughtCharmError -from scenario.state import Event, Relation, State, _CharmSpec +from scenario.state import Relation, State, _CharmSpec, _Event def charm_type(): @@ -56,7 +56,9 @@ class MyEvt(EventBase): ) with runtime.exec( - state=State(), event=Event("bar"), context=Context(my_charm_type, meta=meta) + state=State(), + event=_Event("bar"), + context=Context(my_charm_type, meta=meta), ) as ops: pass @@ -84,7 +86,7 @@ def test_unit_name(app_name, unit_id): with runtime.exec( state=State(), - event=Event("start"), + event=_Event("start"), context=Context(my_charm_type, meta=meta), ) as ops: assert ops.charm.unit.name == f"{app_name}/{unit_id}" @@ -105,7 +107,7 @@ def test_env_cleanup_on_charm_error(): with pytest.raises(UncaughtCharmError): with runtime.exec( state=State(), - event=Event("box_relation_changed", relation=Relation("box")), + event=_Event("box_relation_changed", relation=Relation("box")), context=Context(my_charm_type, meta=meta), ): assert os.getenv("JUJU_REMOTE_APP") From a999380f8e37fd04e9ea73bb8267821a0f67751e Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 11:00:03 +1200 Subject: [PATCH 07/23] Remove the old shortcuts on state components. --- scenario/state.py | 66 ------------------------------- tests/helpers.py | 3 ++ tests/test_consistency_checker.py | 16 -------- tests/test_e2e/test_deferred.py | 8 ++-- tests/test_e2e/test_pebble.py | 4 +- 5 files changed, 9 insertions(+), 88 deletions(-) diff --git a/scenario/state.py b/scenario/state.py index a08163ac..7b02cb12 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -358,46 +358,6 @@ def _validate_databag(self, databag: dict): f"found a value of type {type(v)}", ) - @property - def changed_event(self) -> "_Event": - """Sugar to generate a -relation-changed event.""" - return _Event( - path=normalize_name(self.endpoint + "-relation-changed"), - relation=cast("AnyRelation", self), - ) - - @property - def joined_event(self) -> "_Event": - """Sugar to generate a -relation-joined event.""" - return _Event( - path=normalize_name(self.endpoint + "-relation-joined"), - relation=cast("AnyRelation", self), - ) - - @property - def created_event(self) -> "_Event": - """Sugar to generate a -relation-created event.""" - return _Event( - path=normalize_name(self.endpoint + "-relation-created"), - relation=cast("AnyRelation", self), - ) - - @property - def departed_event(self) -> "_Event": - """Sugar to generate a -relation-departed event.""" - return _Event( - path=normalize_name(self.endpoint + "-relation-departed"), - relation=cast("AnyRelation", self), - ) - - @property - def broken_event(self) -> "_Event": - """Sugar to generate a -relation-broken event.""" - return _Event( - path=normalize_name(self.endpoint + "-relation-broken"), - relation=cast("AnyRelation", self), - ) - _DEFAULT_IP = " 192.0.2.0" DEFAULT_JUJU_DATABAG = { @@ -656,16 +616,6 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated pebble filesystem in this context.""" return ctx._get_container_root(self.name) - @property - def pebble_ready_event(self): - """Sugar to generate a -pebble-ready event.""" - if not self.can_connect: - logger.warning( - "you **can** fire pebble-ready while the container cannot connect, " - "but that's most likely not what you want.", - ) - return _Event(path=normalize_name(self.name + "-pebble-ready"), container=self) - _RawStatusLiteral = Literal[ "waiting", @@ -792,22 +742,6 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated filesystem root in this context.""" return ctx._get_storage_root(self.name, self.index) - @property - def attached_event(self) -> "_Event": - """Sugar to generate a -storage-attached event.""" - return _Event( - path=normalize_name(self.name + "-storage-attached"), - storage=self, - ) - - @property - def detaching_event(self) -> "_Event": - """Sugar to generate a -storage-detached event.""" - return _Event( - path=normalize_name(self.name + "-storage-detaching"), - storage=self, - ) - @dataclasses.dataclass(frozen=True) class State: diff --git a/tests/helpers.py b/tests/helpers.py index 999c35e7..8da96499 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -53,6 +53,9 @@ def trigger( if event.startswith("relation_"): assert len(state.relations) == 1, "shortcut only works with one relation" event = getattr(ctx.on, event)(state.relations[0]) + elif event.startswith("pebble_"): + assert len(state.containers) == 1, "shortcut only works with one container" + event = getattr(ctx.on, event)(state.containers[0]) else: event = getattr(ctx.on, event)() with ctx.manager(event, state=state) as mgr: diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 2d63c4de..7bbd8204 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -319,15 +319,6 @@ def test_dupe_containers_inconsistent(): ) -def test_container_pebble_evt_consistent(): - container = Container("foo-bar-baz") - assert_consistent( - State(containers=[container]), - container.pebble_ready_event, - _CharmSpec(MyCharm, {"containers": {"foo-bar-baz": {}}}), - ) - - def test_action_not_in_meta_inconsistent(): action = Action("foo", params={"bar": "baz"}) assert_inconsistent( @@ -448,13 +439,6 @@ def test_storage_event(): MyCharm, meta={"name": "rupert", "storage": {"foo": {"type": "filesystem"}}} ), ) - assert_consistent( - State(storage=[storage]), - storage.attached_event, - _CharmSpec( - MyCharm, meta={"name": "rupert", "storage": {"foo": {"type": "filesystem"}}} - ), - ) def test_storage_states(): diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index c5394f14..fbd1a05a 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -12,7 +12,7 @@ from ops.framework import Framework from scenario import Context -from scenario.state import Container, DeferredEvent, Relation, State, deferred +from scenario.state import Container, _Event, Relation, State, deferred from tests.helpers import trigger CHARM_CALLED = 0 @@ -79,7 +79,7 @@ def test_deferred_relation_event_without_relation_raises(mycharm): def test_deferred_relation_evt(mycharm): rel = Relation(endpoint="foo", remote_app_name="remote") - evt1 = rel.changed_event.deferred(handler=mycharm._on_event) + evt1 = _Event("foo_relation_changed", relation=rel).deferred(handler=mycharm._on_event) evt2 = deferred( event="foo_relation_changed", handler=mycharm._on_event, @@ -91,7 +91,7 @@ def test_deferred_relation_evt(mycharm): def test_deferred_workload_evt(mycharm): ctr = Container("foo") - evt1 = ctr.pebble_ready_event.deferred(handler=mycharm._on_event) + evt1 = _Event("foo_pebble_ready", container=ctr).deferred(handler=mycharm._on_event) evt2 = deferred(event="foo_pebble_ready", handler=mycharm._on_event, container=ctr) assert asdict(evt2) == asdict(evt1) @@ -175,7 +175,7 @@ def test_deferred_workload_event(mycharm): out = trigger( State( containers=[ctr], - deferred=[ctr.pebble_ready_event.deferred(handler=mycharm._on_event)], + deferred=[_Event("foo_pebble_ready", container=ctr).deferred(handler=mycharm._on_event)], ), "start", mycharm, diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index cca442dd..45c1b348 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -223,7 +223,7 @@ def callback(self: CharmBase): State(containers=[container]), charm_type=charm_cls, meta={"name": "foo", "containers": {"foo": {}}}, - event=container.pebble_ready_event, + event="pebble_ready", post_event=callback, ) @@ -290,7 +290,7 @@ def _on_ready(self, event): State(containers=[container]), charm_type=PlanCharm, meta={"name": "foo", "containers": {"foo": {}}}, - event=container.pebble_ready_event, + event="pebble_ready", ) serv = lambda name, obj: pebble.Service(name, raw=obj) From 25fc7353d8cf2875ad1e610ad14165423ebeea4f Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 11:10:36 +1200 Subject: [PATCH 08/23] Remove old event properties from Secret. --- scenario/context.py | 16 +++++++++++ scenario/state.py | 38 ------------------------- tests/test_context_on.py | 12 ++++---- tests/test_e2e/test_secrets.py | 52 ++++++++++++++++++++-------------- 4 files changed, 52 insertions(+), 66 deletions(-) diff --git a/scenario/context.py b/scenario/context.py index 70bcff7b..f7808f01 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -222,19 +222,35 @@ def leader_elected(): @staticmethod def secret_changed(secret: Secret): + if secret.owner: + raise ValueError( + "This unit will never receive secret-changed for a secret it owns.", + ) return _Event("secret_changed", secret=secret) @staticmethod def secret_expired(secret: Secret, *, revision: int): + if not secret.owner: + raise ValueError( + "This unit will never receive secret-expire for a secret it does not own.", + ) # TODO: Could we have a default revision? return _Event("secret_expired", secret=secret, secret_revision=revision) @staticmethod def secret_rotate(secret: Secret = None): + if not secret.owner: + raise ValueError( + "This unit will never receive secret-rotate for a secret it does not own.", + ) return _Event("secret_rotate", secret=secret) @staticmethod def secret_remove(secret: Secret, *, revision: int): + if not secret.owner: + raise ValueError( + "This unit will never receive secret-removed for a secret it does not own.", + ) # TODO: Could we have a default revision? return _Event("secret_remove", secret=secret, secret_revision=revision) diff --git a/scenario/state.py b/scenario/state.py index 7b02cb12..ae7d739b 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -145,44 +145,6 @@ class Secret: expire: Optional[datetime.datetime] = None rotate: Optional[SecretRotate] = None - # consumer-only events - @property - def changed_event(self): - """Sugar to generate a secret-changed event.""" - if self.owner: - raise ValueError( - "This unit will never receive secret-changed for a secret it owns.", - ) - return _Event("secret_changed", secret=self) - - # owner-only events - @property - def rotate_event(self): - """Sugar to generate a secret-rotate event.""" - if not self.owner: - raise ValueError( - "This unit will never receive secret-rotate for a secret it does not own.", - ) - return _Event("secret_rotate", secret=self) - - @property - def expired_event(self): - """Sugar to generate a secret-expired event.""" - if not self.owner: - raise ValueError( - "This unit will never receive secret-expire for a secret it does not own.", - ) - return _Event("secret_expire", secret=self) - - @property - def remove_event(self): - """Sugar to generate a secret-remove event.""" - if not self.owner: - raise ValueError( - "This unit will never receive secret-removed for a secret it does not own.", - ) - return _Event("secret_removed", secret=self) - def _set_revision(self, revision: int): """Set a new tracked revision.""" # bypass frozen dataclass diff --git a/tests/test_context_on.py b/tests/test_context_on.py index e2d91a82..fe4b8153 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -71,15 +71,15 @@ def test_simple_events(event_name, event_kind): @pytest.mark.parametrize("as_kwarg", [True, False]) @pytest.mark.parametrize( - "event_name,event_kind", + "event_name,event_kind,owner", [ - ("secret_changed", ops.SecretChangedEvent), - ("secret_rotate", ops.SecretRotateEvent), + ("secret_changed", ops.SecretChangedEvent, None), + ("secret_rotate", ops.SecretRotateEvent, "app"), ], ) -def test_simple_secret_events(as_kwarg, event_name, event_kind): +def test_simple_secret_events(as_kwarg, event_name, event_kind, owner): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - secret = scenario.Secret("secret:123", {0: {"password": "xxxx"}}, owner=None) + secret = scenario.Secret("secret:123", {0: {"password": "xxxx"}}, owner=owner) state_in = scenario.State(secrets=[secret]) # These look like: # ctx.run(ctx.on.secret_changed(secret=secret), state) @@ -110,7 +110,7 @@ def test_simple_secret_events(as_kwarg, event_name, event_kind): def test_revision_secret_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) secret = scenario.Secret( - "secret:123", {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, owner=None + "secret:123", {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, owner="app", ) state_in = scenario.State(secrets=[secret]) # These look like: diff --git a/tests/test_e2e/test_secrets.py b/tests/test_e2e/test_secrets.py index d0e6dabb..95186444 100644 --- a/tests/test_e2e/test_secrets.py +++ b/tests/test_e2e/test_secrets.py @@ -120,30 +120,38 @@ def test_get_secret_owner_peek_update(mycharm, owner): @pytest.mark.parametrize("owner", ("app", "unit")) def test_secret_changed_owner_evt_fails(mycharm, owner): + ctx = Context(mycharm, meta={"name": "local"}) + secret = Secret( + id="foo", + contents={ + 0: {"a": "b"}, + 1: {"a": "c"}, + }, + owner=owner, + ) with pytest.raises(ValueError): - _ = Secret( - id="foo", - contents={ - 0: {"a": "b"}, - 1: {"a": "c"}, - }, - owner=owner, - ).changed_event - - -@pytest.mark.parametrize("evt_prefix", ("rotate", "expired", "remove")) -def test_consumer_events_failures(mycharm, evt_prefix): + _ = ctx.on.secret_changed(secret) + + +@pytest.mark.parametrize("evt_suffix,revision", [ + ("rotate", None), + ("expired", 1), + ("remove", 1), + ]) +def test_consumer_events_failures(mycharm, evt_suffix, revision): + ctx = Context(mycharm, meta={"name": "local"}) + secret = Secret( + id="foo", + contents={ + 0: {"a": "b"}, + 1: {"a": "c"}, + }, + ) + kwargs = {"secret": secret} + if revision is not None: + kwargs["revision"] = revision with pytest.raises(ValueError): - _ = getattr( - Secret( - id="foo", - contents={ - 0: {"a": "b"}, - 1: {"a": "c"}, - }, - ), - evt_prefix + "_event", - ) + _ = getattr(ctx.on, f"secret_{evt_suffix}")(**kwargs) @pytest.mark.parametrize("app", (True, False)) From 428b3c254d415ab43c7091e726f18f7f693e5d07 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 11:10:51 +1200 Subject: [PATCH 09/23] Style fixes. --- tests/test_context_on.py | 4 +++- tests/test_e2e/test_deferred.py | 12 +++++++++--- tests/test_e2e/test_secrets.py | 13 ++++++++----- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index fe4b8153..2088a815 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -110,7 +110,9 @@ def test_simple_secret_events(as_kwarg, event_name, event_kind, owner): def test_revision_secret_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) secret = scenario.Secret( - "secret:123", {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, owner="app", + "secret:123", + {42: {"password": "yyyy"}, 43: {"password": "xxxx"}}, + owner="app", ) state_in = scenario.State(secrets=[secret]) # These look like: diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index fbd1a05a..bdbd619a 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -12,7 +12,7 @@ from ops.framework import Framework from scenario import Context -from scenario.state import Container, _Event, Relation, State, deferred +from scenario.state import Container, Relation, State, _Event, deferred from tests.helpers import trigger CHARM_CALLED = 0 @@ -79,7 +79,9 @@ def test_deferred_relation_event_without_relation_raises(mycharm): def test_deferred_relation_evt(mycharm): rel = Relation(endpoint="foo", remote_app_name="remote") - evt1 = _Event("foo_relation_changed", relation=rel).deferred(handler=mycharm._on_event) + evt1 = _Event("foo_relation_changed", relation=rel).deferred( + handler=mycharm._on_event + ) evt2 = deferred( event="foo_relation_changed", handler=mycharm._on_event, @@ -175,7 +177,11 @@ def test_deferred_workload_event(mycharm): out = trigger( State( containers=[ctr], - deferred=[_Event("foo_pebble_ready", container=ctr).deferred(handler=mycharm._on_event)], + deferred=[ + _Event("foo_pebble_ready", container=ctr).deferred( + handler=mycharm._on_event + ) + ], ), "start", mycharm, diff --git a/tests/test_e2e/test_secrets.py b/tests/test_e2e/test_secrets.py index 95186444..681681fb 100644 --- a/tests/test_e2e/test_secrets.py +++ b/tests/test_e2e/test_secrets.py @@ -133,11 +133,14 @@ def test_secret_changed_owner_evt_fails(mycharm, owner): _ = ctx.on.secret_changed(secret) -@pytest.mark.parametrize("evt_suffix,revision", [ - ("rotate", None), - ("expired", 1), - ("remove", 1), - ]) +@pytest.mark.parametrize( + "evt_suffix,revision", + [ + ("rotate", None), + ("expired", 1), + ("remove", 1), + ], +) def test_consumer_events_failures(mycharm, evt_suffix, revision): ctx = Context(mycharm, meta={"name": "local"}) secret = Secret( From 03c9a210b5544dbdeb0ffc8fd7a36ce99ddf4d24 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 18:38:53 +1200 Subject: [PATCH 10/23] Move the checks that were on binding to the consistency checker. --- scenario/consistency_checker.py | 36 +++++++++++++--- tests/test_consistency_checker.py | 71 +++++++++++++++++++++++++++++-- tests/test_context_on.py | 18 ++++++++ tests/test_e2e/test_event_bind.py | 61 -------------------------- 4 files changed, 115 insertions(+), 71 deletions(-) delete mode 100644 tests/test_e2e/test_event_bind.py diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 89002d67..bf6d3d3d 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -121,6 +121,7 @@ def check_event_consistency( *, event: "_Event", charm_spec: "_CharmSpec", + state: "State", **_kwargs, # noqa: U101 ) -> Results: """Check the internal consistency of the Event data structure. @@ -141,16 +142,16 @@ def check_event_consistency( ) if event._is_relation_event: - _check_relation_event(charm_spec, event, errors, warnings) + _check_relation_event(charm_spec, event, state, errors, warnings) if event._is_workload_event: - _check_workload_event(charm_spec, event, errors, warnings) + _check_workload_event(charm_spec, event, state, errors, warnings) if event._is_action_event: - _check_action_event(charm_spec, event, errors, warnings) + _check_action_event(charm_spec, event, state, errors, warnings) if event._is_storage_event: - _check_storage_event(charm_spec, event, errors, warnings) + _check_storage_event(charm_spec, event, state, errors, warnings) return Results(errors, warnings) @@ -158,6 +159,7 @@ def check_event_consistency( def _check_relation_event( charm_spec: _CharmSpec, # noqa: U100 event: "_Event", + state: "State", errors: List[str], warnings: List[str], # noqa: U100 ): @@ -172,11 +174,16 @@ def _check_relation_event( f"relation event should start with relation endpoint name. {event.name} does " f"not start with {event.relation.endpoint}.", ) + if event.relation not in state.relations: + errors.append( + f"cannot emit {event.name} because relation {event.relation.id} is not in the state.", + ) def _check_workload_event( charm_spec: _CharmSpec, # noqa: U100 event: "_Event", + state: "State", errors: List[str], warnings: List[str], # noqa: U100 ): @@ -190,11 +197,21 @@ def _check_workload_event( f"workload event should start with container name. {event.name} does " f"not start with {event.container.name}.", ) + if event.container not in state.containers: + errors.append( + f"cannot emit {event.name} because container {event.container.name} " + f"is not in the state.", + ) + if not event.container.can_connect: + warnings.append( + "it's strange to have a workload event when can_connect is False", + ) def _check_action_event( charm_spec: _CharmSpec, event: "_Event", + state: "State", # noqa: U100 errors: List[str], warnings: List[str], ): @@ -224,6 +241,7 @@ def _check_action_event( def _check_storage_event( charm_spec: _CharmSpec, event: "_Event", + state: "State", errors: List[str], warnings: List[str], # noqa: U100 ): @@ -245,6 +263,11 @@ def _check_storage_event( f"storage event {event.name} refers to storage {storage.name} " f"which is not declared in the charm metadata (metadata.yaml) under 'storage'.", ) + elif storage not in state.storage: + errors.append( + f"cannot emit {event.name} because storage {storage.name} " + f"is not in the state.", + ) def _check_action_param_types( @@ -406,9 +429,10 @@ def check_secrets_consistency( if not event._is_secret_event: return Results(errors, []) - if not state.secrets: + if event.secret not in state.secrets: + secret_key = event.secret.id if event.secret.id else event.secret.label errors.append( - "the event being processed is a secret event; but the state has no secrets.", + f"cannot emit {event.name} because secret {secret_key} is not in the state.", ) elif juju_version < (3,): errors.append( diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 7bbd8204..1100ef9b 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -91,6 +91,20 @@ def test_container_in_state_but_no_container_in_meta(): ) +def test_container_not_in_state(): + container = Container("bar") + assert_inconsistent( + State(), + _Event("bar_pebble_ready", container=container), + _CharmSpec(MyCharm, {"containers": {"bar": {}}}), + ) + assert_consistent( + State(containers=[container]), + _Event("bar_pebble_ready", container=container), + _CharmSpec(MyCharm, {"containers": {"bar": {}}}), + ) + + def test_evt_bad_container_name(): assert_inconsistent( State(), @@ -111,9 +125,10 @@ def test_evt_bad_relation_name(suffix): _Event(f"foo{suffix}", relation=Relation("bar")), _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "xxx"}}}), ) + relation = Relation("bar") assert_consistent( - State(relations=[Relation("bar")]), - _Event(f"bar{suffix}", relation=Relation("bar")), + State(relations=[relation]), + _Event(f"bar{suffix}", relation=relation), _CharmSpec(MyCharm, {"requires": {"bar": {"interface": "xxx"}}}), ) @@ -121,9 +136,10 @@ def test_evt_bad_relation_name(suffix): @pytest.mark.parametrize("suffix", RELATION_EVENTS_SUFFIX) def test_evt_no_relation(suffix): assert_inconsistent(State(), _Event(f"foo{suffix}"), _CharmSpec(MyCharm, {})) + relation = Relation("bar") assert_consistent( - State(relations=[Relation("bar")]), - _Event(f"bar{suffix}", relation=Relation("bar")), + State(relations=[relation]), + _Event(f"bar{suffix}", relation=relation), _CharmSpec(MyCharm, {"requires": {"bar": {"interface": "xxx"}}}), ) @@ -256,6 +272,20 @@ def test_secrets_jujuv_bad(good_v): ) +def test_secret_not_in_state(): + secret = Secret("secret:foo", {"a": "b"}) + assert_inconsistent( + State(), + _Event("secret_changed", secret=secret), + _CharmSpec(MyCharm, {}), + ) + assert_consistent( + State(secrets=[secret]), + _Event("secret_changed", secret=secret), + _CharmSpec(MyCharm, {}), + ) + + def test_peer_relation_consistency(): assert_inconsistent( State(relations=[Relation("foo")]), @@ -310,6 +340,19 @@ def test_relation_sub_inconsistent(): _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), ) +def test_relation_not_in_state(): + relation = Relation("foo") + assert_inconsistent( + State(), + _Event("foo_relation_changed", relation=relation), + _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), + ) + assert_consistent( + State(relations=[relation]), + _Event("foo_relation_changed", relation=relation), + _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), + ) + def test_dupe_containers_inconsistent(): assert_inconsistent( @@ -473,6 +516,26 @@ def test_storage_states(): ) +def test_storage_not_in_state(): + storage = Storage("foo") + assert_inconsistent( + State(), + _Event("foo_storage_attached", storage=storage), + _CharmSpec( + MyCharm, + meta={"name": "sam", "storage": {"foo": {"type": "filesystem"}}}, + ), + ) + assert_consistent( + State(storage=[storage]), + _Event("foo_storage_attached", storage=storage), + _CharmSpec( + MyCharm, + meta={"name": "sam", "storage": {"foo": {"type": "filesystem"}}}, + ), + ) + + def test_resource_states(): # happy path assert_consistent( diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 2088a815..be8c70b5 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -1,3 +1,5 @@ +import copy + import ops import pytest @@ -238,6 +240,22 @@ def test_relation_app_events(as_kwarg, event_name, event_kind): assert event.unit is None +def test_relation_complex_name(): + meta = copy.deepcopy(META) + meta["requires"]["foo-bar-baz"] = {"interface": "another-one"} + ctx = scenario.Context(ContextCharm, meta=meta, actions=ACTIONS) + relation = scenario.Relation("foo-bar-baz") + state_in = scenario.State(relations=[relation]) + with ctx.manager(ctx.on.relation_created(relation), state_in) as mgr: + mgr.run() + assert len(mgr.charm.observed) == 2 + event = mgr.charm.observed[0] + assert isinstance(event, ops.RelationCreatedEvent) + assert event.relation.id == relation.id + assert event.app.name == relation.remote_app_name + assert event.unit is None + + @pytest.mark.parametrize("event_name", ["relation_created", "relation_broken"]) def test_relation_events_as_positional_arg(event_name): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) diff --git a/tests/test_e2e/test_event_bind.py b/tests/test_e2e/test_event_bind.py deleted file mode 100644 index c0d9abfb..00000000 --- a/tests/test_e2e/test_event_bind.py +++ /dev/null @@ -1,61 +0,0 @@ -import pytest - -from scenario import Container, Relation, Secret, State - - -def test_bind_relation(): - event = _Event("foo-relation-changed") - foo_relation = Relation("foo") - state = State(relations=[foo_relation]) - assert event.bind(state).relation is foo_relation - - -def test_bind_relation_complex_name(): - event = _Event("foo-bar-baz-relation-changed") - foo_relation = Relation("foo_bar_baz") - state = State(relations=[foo_relation]) - assert event.bind(state).relation is foo_relation - - -def test_bind_relation_notfound(): - event = _Event("foo-relation-changed") - state = State(relations=[]) - with pytest.raises(BindFailedError): - event.bind(state) - - -def test_bind_relation_toomany(caplog): - event = _Event("foo-relation-changed") - foo_relation = Relation("foo") - foo_relation1 = Relation("foo") - state = State(relations=[foo_relation, foo_relation1]) - event.bind(state) - assert "too many relations" in caplog.text - - -def test_bind_secret(): - event = _Event("secret-changed") - secret = Secret("foo", {"a": "b"}) - state = State(secrets=[secret]) - assert event.bind(state).secret is secret - - -def test_bind_secret_notfound(): - event = _Event("secret-changed") - state = State(secrets=[]) - with pytest.raises(BindFailedError): - event.bind(state) - - -def test_bind_container(): - event = _Event("foo-pebble-ready") - container = Container("foo") - state = State(containers=[container]) - assert event.bind(state).container is container - - -def test_bind_container_notfound(): - event = _Event("foo-pebble-ready") - state = State(containers=[]) - with pytest.raises(BindFailedError): - event.bind(state) From 3e131a8c7e6032f1e2e1ce18e478c5f7bc819d8c Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 18:46:36 +1200 Subject: [PATCH 11/23] Update rubbish event tests. These are not as valuable without support for emitting custom events directly, but it seems worthwhile to leave them in so that if we add custom event emitting back in the future they're here to build off. --- scenario/ops_main_mock.py | 3 +-- tests/test_e2e/test_rubbish_events.py | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scenario/ops_main_mock.py b/scenario/ops_main_mock.py index 7aeaa60a..433b880f 100644 --- a/scenario/ops_main_mock.py +++ b/scenario/ops_main_mock.py @@ -71,8 +71,7 @@ def _emit_charm_event( ops_logger.debug("Event %s not defined for %s.", event_name, charm) raise NoObserverError( f"Cannot fire {event_name!r} on {owner}: " - f"invalid event (not on charm.on). " - f"Use Context.run_custom instead.", + f"invalid event (not on charm.on).", ) args, kwargs = _get_event_args(charm, event_to_emit) diff --git a/tests/test_e2e/test_rubbish_events.py b/tests/test_e2e/test_rubbish_events.py index b20ae4a9..0656b80c 100644 --- a/tests/test_e2e/test_rubbish_events.py +++ b/tests/test_e2e/test_rubbish_events.py @@ -46,7 +46,7 @@ def _on_event(self, e): @pytest.mark.parametrize("evt_name", ("rubbish", "foo", "bar", "kazoo_pebble_ready")) def test_rubbish_event_raises(mycharm, evt_name): - with pytest.raises(NoObserverError): + with pytest.raises(AttributeError): if evt_name.startswith("kazoo"): os.environ["SCENARIO_SKIP_CONSISTENCY_CHECKS"] = "true" # else it will whine about the container not being in state and meta; @@ -59,14 +59,15 @@ def test_rubbish_event_raises(mycharm, evt_name): @pytest.mark.parametrize("evt_name", ("qux",)) -def test_custom_events_pass(mycharm, evt_name): - trigger(State(), evt_name, mycharm, meta={"name": "foo"}) +def test_custom_events_fail(mycharm, evt_name): + with pytest.raises(AttributeError): + trigger(State(), evt_name, mycharm, meta={"name": "foo"}) # cfr: https://github.com/PietroPasotti/ops-scenario/pull/11#discussion_r1101694961 @pytest.mark.parametrize("evt_name", ("sub",)) def test_custom_events_sub_raise(mycharm, evt_name): - with pytest.raises(RuntimeError): + with pytest.raises(AttributeError): trigger(State(), evt_name, mycharm, meta={"name": "foo"}) From 41c54a243f4ef01466bb0790c31c7ecac57f5186 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 19:36:44 +1200 Subject: [PATCH 12/23] Update tests now that emitting custom events is not possible. --- scenario/consistency_checker.py | 1 + scenario/context.py | 2 +- tests/test_consistency_checker.py | 3 ++- tests/test_emitted_events_util.py | 29 +++++++---------------------- 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index bf6d3d3d..cc9db2b9 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -429,6 +429,7 @@ def check_secrets_consistency( if not event._is_secret_event: return Results(errors, []) + assert event.secret is not None if event.secret not in state.secrets: secret_key = event.secret.id if event.secret.id else event.secret.label errors.append( diff --git a/scenario/context.py b/scenario/context.py index f7808f01..63c3cda9 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -238,7 +238,7 @@ def secret_expired(secret: Secret, *, revision: int): return _Event("secret_expired", secret=secret, secret_revision=revision) @staticmethod - def secret_rotate(secret: Secret = None): + def secret_rotate(secret: Secret): if not secret.owner: raise ValueError( "This unit will never receive secret-rotate for a secret it does not own.", diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 1100ef9b..059e7fc9 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -284,7 +284,7 @@ def test_secret_not_in_state(): _Event("secret_changed", secret=secret), _CharmSpec(MyCharm, {}), ) - + def test_peer_relation_consistency(): assert_inconsistent( @@ -340,6 +340,7 @@ def test_relation_sub_inconsistent(): _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), ) + def test_relation_not_in_state(): relation = Relation("foo") assert_inconsistent( diff --git a/tests/test_emitted_events_util.py b/tests/test_emitted_events_util.py index e2cec7d2..b54c84b4 100644 --- a/tests/test_emitted_events_util.py +++ b/tests/test_emitted_events_util.py @@ -32,31 +32,16 @@ def _on_foo(self, e): pass -def test_capture_custom_evt(): - with capture_events(Foo) as emitted: - trigger(State(), "foo", MyCharm, meta=MyCharm.META) - - assert len(emitted) == 1 - assert isinstance(emitted[0], Foo) - - -def test_capture_custom_evt_nonspecific_capture(): - with capture_events() as emitted: - trigger(State(), "foo", MyCharm, meta=MyCharm.META) - - assert len(emitted) == 1 - assert isinstance(emitted[0], Foo) - - def test_capture_custom_evt_nonspecific_capture_include_fw_evts(): with capture_events(include_framework=True) as emitted: - trigger(State(), "foo", MyCharm, meta=MyCharm.META) + trigger(State(), "start", MyCharm, meta=MyCharm.META) - assert len(emitted) == 4 - assert isinstance(emitted[0], Foo) - assert isinstance(emitted[1], CollectStatusEvent) - assert isinstance(emitted[2], PreCommitEvent) - assert isinstance(emitted[3], CommitEvent) + assert len(emitted) == 5 + assert isinstance(emitted[0], StartEvent) + assert isinstance(emitted[1], Foo) + assert isinstance(emitted[2], CollectStatusEvent) + assert isinstance(emitted[3], PreCommitEvent) + assert isinstance(emitted[4], CommitEvent) def test_capture_juju_evt(): From c01e8ea14e6f73ff76be2437ddadfb067f0b5e60 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 19:53:01 +1200 Subject: [PATCH 13/23] Minor clean-up. --- README.md | 5 +++-- scenario/consistency_checker.py | 3 ++- scenario/context.py | 22 ++++------------------ 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index ff995d6b..eeddec80 100644 --- a/README.md +++ b/README.md @@ -415,7 +415,8 @@ state_in = scenario.State(relations=[ peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}}, )]) -scenario.Context(..., unit_id=1).run(ctx.on.start(), state_in) # invalid: this unit's id cannot be the ID of a peer. +ctx = scenario.Context(..., unit_id=1) +ctx.run(ctx.on.start(), state_in) # invalid: this unit's id cannot be the ID of a peer. ``` @@ -780,7 +781,7 @@ import scenario ctx = scenario.Context(MyCharm) foo_0 = scenario.Storage('foo') # The charm is notified that one of the storages it has requested is ready: -ctx.run(ctx.on.storage_sttached(foo_0), State(storage=[foo_0])) +ctx.run(ctx.on.storage_attached(foo_0), State(storage=[foo_0])) foo_1 = scenario.Storage('foo') # The charm is notified that the other storage is also ready: diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index cc9db2b9..30b95d51 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -204,7 +204,8 @@ def _check_workload_event( ) if not event.container.can_connect: warnings.append( - "it's strange to have a workload event when can_connect is False", + "you **can** fire fire pebble-ready while the container cannot connect, " + "but that's most likely not what you want.", ) diff --git a/scenario/context.py b/scenario/context.py index 63c3cda9..407281f1 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -178,8 +178,6 @@ class _CharmEvents: arguments). """ - # TODO: There are lots of suffix definitions in state - we should be able to re-use those. - @staticmethod def install(): return _Event("install") @@ -234,7 +232,6 @@ def secret_expired(secret: Secret, *, revision: int): raise ValueError( "This unit will never receive secret-expire for a secret it does not own.", ) - # TODO: Could we have a default revision? return _Event("secret_expired", secret=secret, secret_revision=revision) @staticmethod @@ -251,7 +248,6 @@ def secret_remove(secret: Secret, *, revision: int): raise ValueError( "This unit will never receive secret-removed for a secret it does not own.", ) - # TODO: Could we have a default revision? return _Event("secret_remove", secret=secret, secret_revision=revision) @staticmethod @@ -263,13 +259,8 @@ def collect_unit_status(): return _Event("collect_unit_status") @staticmethod - def relation_created(relation: "AnyRelation", *, remote_unit: Optional[int] = None): - # TODO: Does remote_unit make sense here? The old Scenario API had it. - return _Event( - f"{relation.endpoint}_relation_created", - relation=relation, - relation_remote_unit_id=remote_unit, - ) + def relation_created(relation: "AnyRelation"): + return _Event(f"{relation.endpoint}_relation_created", relation=relation) @staticmethod def relation_joined(relation: "AnyRelation", *, remote_unit: Optional[int] = None): @@ -302,13 +293,8 @@ def relation_departed( ) @staticmethod - def relation_broken(relation: "AnyRelation", *, remote_unit: Optional[int] = None): - # TODO: Does remote_unit make sense here? The old Scenario API had it. - return _Event( - f"{relation.endpoint}_relation_broken", - relation=relation, - relation_remote_unit_id=remote_unit, - ) + def relation_broken(relation: "AnyRelation"): + return _Event(f"{relation.endpoint}_relation_broken", relation=relation) @staticmethod def storage_attached(storage: Storage): From 276cb76f022735343800bda47440b844511083e0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 17:51:06 +1200 Subject: [PATCH 14/23] Fix typo found in review. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index eeddec80..77e07fe0 100644 --- a/README.md +++ b/README.md @@ -210,7 +210,7 @@ import ops import scenario # ... -ctx.run(ctx.start(), scenario.State(unit_status=ops.ActiveStatus('foo'))) +ctx.run(ctx.on.start(), scenario.State(unit_status=ops.ActiveStatus('foo'))) assert ctx.unit_status_history == [ ops.ActiveStatus('foo'), # now the first status is active: 'foo'! # ... From 446410bd65b5233459ef8b0071524218f7d4c8fe Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 17:57:12 +1200 Subject: [PATCH 15/23] Fix tests for relation.unit. --- tests/test_e2e/test_relations.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/test_e2e/test_relations.py b/tests/test_e2e/test_relations.py index bd2e9751..e72f754c 100644 --- a/tests/test_e2e/test_relations.py +++ b/tests/test_e2e/test_relations.py @@ -156,8 +156,14 @@ def callback(charm: CharmBase, e): @pytest.mark.parametrize( - "evt_name", - ("changed", "broken", "departed", "joined", "created"), + "evt_name,has_unit", + [ + ("changed", True), + ("broken", False), + ("departed", True), + ("joined", True), + ("created", False), + ], ) @pytest.mark.parametrize( "remote_app_name", @@ -167,7 +173,9 @@ def callback(charm: CharmBase, e): "remote_unit_id", (0, 1), ) -def test_relation_events_attrs(mycharm, evt_name, remote_app_name, remote_unit_id): +def test_relation_events_attrs( + mycharm, evt_name, has_unit, remote_app_name, remote_unit_id +): relation = Relation( endpoint="foo", interface="foo", remote_app_name=remote_app_name ) @@ -177,7 +185,8 @@ def callback(charm: CharmBase, event): return assert event.app - assert event.unit + if not isinstance(event, (RelationCreatedEvent, RelationBrokenEvent)): + assert event.unit if isinstance(event, RelationDepartedEvent): assert event.departing_unit @@ -193,9 +202,10 @@ def callback(charm: CharmBase, event): }, ) state = State(relations=[relation]) - event = getattr(ctx.on, f"relation_{evt_name}")( - relation, remote_unit=remote_unit_id - ) + kwargs = {} + if has_unit: + kwargs["remote_unit"] = remote_unit_id + event = getattr(ctx.on, f"relation_{evt_name}")(relation, **kwargs) ctx.run(event, state=state) From 61160dac32d3aae2a76cdd2a8fb7ac91323789d3 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 17:40:48 +1200 Subject: [PATCH 16/23] Test the code in the README. --- .pre-commit-config.yaml | 1 - README.md | 215 +++++++++------------------------------ tests/readme-conftest.py | 56 ++++++++++ tox.ini | 17 ++++ 4 files changed, 122 insertions(+), 167 deletions(-) create mode 100644 tests/readme-conftest.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 947fcf5c..0f8dfeca 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,7 +16,6 @@ repos: rev: v3.1.0 hooks: - id: add-trailing-comma - args: [--py36-plus] - repo: https://github.com/asottile/pyupgrade rev: v3.3.1 hooks: diff --git a/README.md b/README.md index 77e07fe0..fad09daa 100644 --- a/README.md +++ b/README.md @@ -82,14 +82,6 @@ available. The charm has no config, no relations, no leadership, and its status With that, we can write the simplest possible scenario test: ```python -import ops -import scenario - - -class MyCharm(ops.CharmBase): - pass - - def test_scenario_base(): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) out = ctx.run(ctx.on.start(), scenario.State()) @@ -99,9 +91,7 @@ def test_scenario_base(): Now let's start making it more complicated. Our charm sets a special state if it has leadership on 'start': ```python -import ops import pytest -import scenario class MyCharm(ops.CharmBase): @@ -133,9 +123,6 @@ sets the expected unit/application status. We have seen a simple example above i charm transitions through a sequence of statuses? ```python -import ops - - # charm code: def _on_event(self, _event): self.unit.status = ops.MaintenanceStatus('determining who the ruler is...') @@ -174,11 +161,6 @@ context. You can verify that the charm has followed the expected path by checking the unit/app status history like so: ```python -import ops -import scenario -from charm import MyCharm - - def test_statuses(): ctx = scenario.Context(MyCharm, meta={"name": "foo"}) out = ctx.run(ctx.on.start(), scenario.State(leader=False)) @@ -187,7 +169,7 @@ def test_statuses(): ops.MaintenanceStatus('determining who the ruler is...'), ops.WaitingStatus('checking this is right...'), ] - assert out.unit_status == ops.ActiveStatus("I am ruled"), + assert out.unit_status == ops.ActiveStatus("I am ruled") # similarly you can check the app status history: assert ctx.app_status_history == [ @@ -206,10 +188,16 @@ If you want to simulate a situation in which the charm already has seen some eve Unknown (the default status every charm is born with), you will have to pass the 'initial status' to State. ```python -import ops -import scenario +class MyCharm(ops.CharmBase): + def __init__(self, framework): + super().__init__(framework) + framework.observe(self.on.start, self._on_start) + + def _on_start(self, event): + self.model.unit.status = ops.ActiveStatus("foo") # ... +ctx = scenario.Context(MyCharm, meta={"name": "foo"}) ctx.run(ctx.on.start(), scenario.State(unit_status=ops.ActiveStatus('foo'))) assert ctx.unit_status_history == [ ops.ActiveStatus('foo'), # now the first status is active: 'foo'! @@ -223,10 +211,9 @@ Using a similar api to `*_status_history`, you can assert that the charm has set hook execution: ```python -import scenario - # ... -ctx: scenario.Context +ctx = scenario.Context(HistoryCharm, meta={"name": "foo"}) +ctx.run("start", scenario.State()) assert ctx.workload_version_history == ['1', '1.2', '1.5'] # ... ``` @@ -241,10 +228,6 @@ given Juju event triggering (say, 'start'), a specific chain of events is emitte resulting state, black-box as it is, gives little insight into how exactly it was obtained. ```python -import ops -import scenario - - def test_foo(): ctx = scenario.Context(...) ctx.run(ctx.on.start(), ...) @@ -259,8 +242,6 @@ You can configure what events will be captured by passing the following argument For example: ```python -import scenario - def test_emitted_full(): ctx = scenario.Context( MyCharm, @@ -288,14 +269,13 @@ This context manager allows you to intercept any events emitted by the framework Usage: ```python -import ops -import scenario +import scenario.capture_events -with capture_events() as emitted: - ctx = scenario.Context(...) +with scenario.capture_events.capture_events() as emitted: + ctx = scenario.Context(SimpleCharm, meta={"name": "capture"}) state_out = ctx.run( ctx.on.update_status(), - scenario.State(deferred=[scenario.DeferredEvent("start", ...)]) + scenario.State(deferred=[scenario.deferred("start", SimpleCharm._on_start)]) ) # deferred events get reemitted first @@ -310,9 +290,9 @@ assert isinstance(emitted[1], ops.UpdateStatusEvent) You can filter events by type like so: ```python -import ops +import scenario.capture_events -with capture_events(ops.StartEvent, ops.RelationEvent) as emitted: +with scenario.capture_events.capture_events(ops.StartEvent, ops.RelationEvent) as emitted: # capture all `start` and `*-relation-*` events. pass ``` @@ -330,10 +310,6 @@ Configuration: You can write scenario tests to verify the shape of relation data: ```python -import ops -import scenario - - # This charm copies over remote app data to local unit data class MyCharm(ops.CharmBase): ... @@ -395,8 +371,6 @@ have `remote_app_name` or `remote_app_data` arguments. Also, it talks in terms o - `Relation.remote_units_data` maps to `PeerRelation.peers_data` ```python -import scenario - relation = scenario.PeerRelation( endpoint="peers", peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}}, @@ -407,15 +381,21 @@ be mindful when using `PeerRelation` not to include **"this unit"**'s ID in `pee be flagged by the Consistency Checker: ```python -import scenario - state_in = scenario.State(relations=[ scenario.PeerRelation( endpoint="peers", peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}}, )]) -ctx = scenario.Context(..., unit_id=1) +meta = { + "name": "invalid", + "peers": { + "peers": { + "interface": "foo", + } + } +} +ctx = scenario.Context(ops.CharmBase, meta=meta, unit_id=1) ctx.run(ctx.on.start(), state_in) # invalid: this unit's id cannot be the ID of a peer. @@ -433,8 +413,6 @@ Because of that, `SubordinateRelation`, compared to `Relation`, always talks in - `Relation.remote_units_data` becomes `SubordinateRelation.remote_unit_data` (a single databag instead of a mapping from unit IDs to databags) ```python -import scenario - relation = scenario.SubordinateRelation( endpoint="peers", remote_unit_data={"foo": "bar"}, @@ -450,8 +428,6 @@ If you want to trigger relation events, the easiest way to do so is get a hold o event from one of its aptly-named properties: ```python -import scenario - relation = scenario.Relation(endpoint="foo", interface="bar") changed_event = relation.changed_event joined_event = relation.joined_event @@ -461,8 +437,6 @@ joined_event = relation.joined_event This is in fact syntactic sugar for: ```python -import scenario - relation = scenario.Relation(endpoint="foo", interface="bar") changed_event = scenario.Event('foo-relation-changed', relation=relation) ``` @@ -490,7 +464,7 @@ import dataclasses import scenario.state rel = scenario.Relation('foo') -rel2 = dataclasses.replace(rel, local_app_data={"foo": "bar"}, relation_id=scenario.state.next_relation_id()) +rel2 = dataclasses.replace(rel, local_app_data={"foo": "bar"}, id=scenario.state.next_relation_id()) assert rel2.id == rel.id + 1 ``` @@ -511,8 +485,6 @@ The `remote_unit_id` will default to the first ID found in the relation's `remot writing is close to that domain, you should probably override it and pass it manually. ```python -import scenario - relation = scenario.Relation(endpoint="foo", interface="bar") remote_unit_2_is_joining_event = relation.joined_event(remote_unit_id=2) @@ -532,8 +504,6 @@ On top of the relation-provided network bindings, a charm can also define some ` If you want to, you can override any of these relation or extra-binding associated networks with a custom one by passing it to `State.networks`. ```python -import scenario - state = scenario.State(networks={ 'foo': scenario.Network.default(private_address='192.0.2.1') }) @@ -552,8 +522,6 @@ To give the charm access to some containers, you need to pass them to the input An example of a state including some containers: ```python -import scenario - state = scenario.State(containers=[ scenario.Container(name="foo", can_connect=True), scenario.Container(name="bar", can_connect=False) @@ -569,14 +537,12 @@ You can configure a container to have some files in it: ```python import pathlib -import scenario - local_file = pathlib.Path('/path/to/local/real/file.txt') container = scenario.Container( name="foo", can_connect=True, - mounts={'local': Mount('/local/share/config.yaml', local_file)} + mounts={'local': scenario.Mount('/local/share/config.yaml', local_file)} ) state = scenario.State(containers=[container]) ``` @@ -597,9 +563,6 @@ data and passing it to the charm via the container. ```python import tempfile -import ops -import scenario - class MyCharm(ops.CharmBase): def __init__(self, framework): @@ -640,10 +603,6 @@ that envvar into the charm's runtime. If the charm writes files to a container (to a location you didn't Mount as a temporary folder you have access to), you will be able to inspect them using the `get_filesystem` api. ```python -import ops -import scenario - - class MyCharm(ops.CharmBase): def __init__(self, framework): super().__init__(framework) @@ -677,9 +636,6 @@ worse issues to deal with. You need to specify, for each possible command the ch result of that would be: its return code, what will be written to stdout/stderr. ```python -import ops -import scenario - LS_LL = """ .rw-rw-r-- 228 ubuntu ubuntu 18 jan 12:05 -- charmcraft.yaml .rw-rw-r-- 497 ubuntu ubuntu 18 jan 12:05 -- config.yaml @@ -723,10 +679,8 @@ If your charm defines `storage` in its metadata, you can use `scenario.Storage` Using the same `get_filesystem` API as `Container`, you can access the temporary directory used by Scenario to mock the filesystem root before and after the scenario runs. ```python -import scenario - # Some charm with a 'foo' filesystem-type storage defined in its metadata: -ctx = scenario.Context(MyCharm) +ctx = scenario.Context(MyCharm, meta=MyCharm.META) storage = scenario.Storage("foo") # Setup storage with some content: @@ -754,7 +708,7 @@ Note that State only wants to know about **attached** storages. A storage which If a charm requests adding more storage instances while handling some event, you can inspect that from the `Context.requested_storage` API. -```python +```python notest # In MyCharm._on_foo: # The charm requests two new "foo" storage instances to be provisioned: self.model.storages.request("foo", 2) @@ -762,10 +716,8 @@ self.model.storages.request("foo", 2) From test code, you can inspect that: -```python -import scenario - -ctx = scenario.Context(MyCharm) +```python notest +ctx = scenario.Context(MyCharm, meta=MyCharm.META) ctx.run(ctx.on.some_event_that_will_cause_on_foo_to_be_called(), scenario.State()) # the charm has requested two 'foo' storages to be provisioned: @@ -776,16 +728,14 @@ Requesting storages has no other consequence in Scenario. In real life, this req So a natural follow-up Scenario test suite for this case would be: ```python -import scenario - -ctx = scenario.Context(MyCharm) +ctx = scenario.Context(MyCharm, meta=MyCharm.META) foo_0 = scenario.Storage('foo') # The charm is notified that one of the storages it has requested is ready: -ctx.run(ctx.on.storage_attached(foo_0), State(storage=[foo_0])) +ctx.run(ctx.on.storage_attached(foo_0), scenario.State(storage=[foo_0])) foo_1 = scenario.Storage('foo') # The charm is notified that the other storage is also ready: -ctx.run(ctx.on.storage_attached(foo_1), State(storage=[foo_0, foo_1])) +ctx.run(ctx.on.storage_attached(foo_1), scenario.State(storage=[foo_0, foo_1])) ``` ## Ports @@ -793,17 +743,12 @@ ctx.run(ctx.on.storage_attached(foo_1), State(storage=[foo_0, foo_1])) Since `ops 2.6.0`, charms can invoke the `open-port`, `close-port`, and `opened-ports` hook tools to manage the ports opened on the host VM/container. Using the `State.opened_ports` API, you can: - simulate a charm run with a port opened by some previous execution -```python -import scenario - -ctx = scenario.Context(MyCharm) -ctx.run(ctx.on.start(), scenario.State(opened_ports=[scenario.Port("tcp", 42)])) +ctx = scenario.Context(MyCharm, meta=MyCharm.META) +ctx.run("start", scenario.State(opened_ports=[scenario.Port("tcp", 42)])) ``` - assert that a charm has called `open-port` or `close-port`: ```python -import scenario - -ctx = scenario.Context(MyCharm) +ctx = scenario.Context(PortCharm, meta=MyCharm.META) state1 = ctx.run(ctx.on.start(), scenario.State()) assert state1.opened_ports == [scenario.Port("tcp", 42)] @@ -816,8 +761,6 @@ assert state2.opened_ports == [] Scenario has secrets. Here's how you use them. ```python -import scenario - state = scenario.State( secrets=[ scenario.Secret( @@ -847,8 +790,6 @@ If this charm does not own the secret, but also it was not granted view rights b To specify a secret owned by this unit (or app): ```python -import scenario - state = scenario.State( secrets=[ scenario.Secret( @@ -865,8 +806,6 @@ state = scenario.State( To specify a secret owned by some other application and give this unit (or app) access to it: ```python -import scenario - state = scenario.State( secrets=[ scenario.Secret( @@ -884,12 +823,6 @@ state = scenario.State( Scenario can simulate StoredState. You can define it on the input side as: ```python -import ops -import scenario - -from ops.charm import CharmBase - - class MyCharmType(ops.CharmBase): my_stored_state = ops.StoredState() @@ -921,13 +854,13 @@ However, when testing, this constraint is unnecessarily strict (and it would als So, the only consistency-level check we enforce in Scenario when it comes to resource is that if a resource is provided in State, it needs to have been declared in the metadata. ```python -import scenario +import pathlib ctx = scenario.Context(MyCharm, meta={'name': 'juliette', "resources": {"foo": {"type": "oci-image"}}}) with ctx.manager("start", scenario.State(resources={'foo': '/path/to/resource.tar'})) as mgr: # If the charm, at runtime, were to call self.model.resources.fetch("foo"), it would get '/path/to/resource.tar' back. path = mgr.charm.model.resources.fetch('foo') - assert path == '/path/to/resource.tar' + assert path == pathlib.Path('/path/to/resource.tar') ``` ## Model @@ -937,12 +870,6 @@ but if you need to set the model name or UUID, you can provide a `scenario.Model to the state: ```python -import ops -import scenario - -class MyCharm(ops.CharmBase): - pass - ctx = scenario.Context(MyCharm, meta={"name": "foo"}) state_in = scenario.State(model=scenario.Model(name="my-model")) out = ctx.run(ctx.on.start(), state_in) @@ -962,11 +889,6 @@ How to test actions with scenario: ## Actions without parameters ```python -import scenario - -from charm import MyCharm - - def test_backup_action(): ctx = scenario.Context(MyCharm) @@ -991,11 +913,6 @@ def test_backup_action(): If the action takes parameters, you'll need to instantiate an `Action`. ```python -import scenario - -from charm import MyCharm - - def test_backup_action(): # Define an action: action = scenario.Action('do_backup', params={'a': 'b'}) @@ -1016,10 +933,7 @@ event in its queue (they would be there because they had been deferred in the pr valid. ```python -import scenario - - -class MyCharm(...): +class MyCharm(ops.CharmBase): ... def _on_update_status(self, event): @@ -1044,14 +958,7 @@ def test_start_on_deferred_update_status(MyCharm): You can also generate the 'deferred' data structure (called a DeferredEvent) from the corresponding Event (and the handler): -```python -import scenario - - -class MyCharm(...): - ... - - +```python continuation deferred_start = scenario.Event('start').deferred(MyCharm._on_start) deferred_install = scenario.Event('install').deferred(MyCharm._on_start) ``` @@ -1060,10 +967,7 @@ On the output side, you can verify that an event that you expect to have been de been deferred. ```python -import scenario - - -class MyCharm(...): +class MyCharm(ops.CharmBase): ... def _on_start(self, event): @@ -1083,10 +987,7 @@ Relation instance they are about. So do they in Scenario. You can use the deferr structure: ```python -import scenario - - -class MyCharm(...): +class MyCharm(ops.CharmBase): ... def _on_foo_relation_changed(self, event): @@ -1107,14 +1008,7 @@ def test_start_on_deferred_update_status(MyCharm): but you can also use a shortcut from the relation event itself: -```python -import scenario - - -class MyCharm(...): - ... - - +```python continuation foo_relation = scenario.Relation('foo') foo_relation.changed_event.deferred(handler=MyCharm._on_foo_relation_changed) ``` @@ -1127,10 +1021,7 @@ given piece of data, or would return this and that _if_ it had been called. Scenario offers a cheekily-named context manager for this use case specifically: -```python -import ops -import scenario - +```python notest from charms.bar.lib_name.v1.charm_lib import CharmLib @@ -1184,16 +1075,12 @@ either inferred from the charm type being passed to `Context` or be passed to it the inferred one. This also allows you to test charms defined on the fly, as in: ```python -import ops -import scenario - - class MyCharmType(ops.CharmBase): pass ctx = scenario.Context(charm_type=MyCharmType, meta={'name': 'my-charm-name'}) -ctx.run(ctx.on.start(), State()) +ctx.run(ctx.on.start(), scenario.State()) ``` A consequence of this fact is that you have no direct control over the temporary directory that we are creating to put the metadata @@ -1202,9 +1089,6 @@ you are passing to `.run()` (because `ops` expects it to be a file...). That is, ```python import tempfile -import ops -import scenario - class MyCharmType(ops.CharmBase): pass @@ -1237,11 +1121,10 @@ the dataclasses `replace` api. ```python import dataclasses -import scenario relation = scenario.Relation('foo', remote_app_data={"1": "2"}) -# make a copy of relation, but with remote_app_data set to {"3", "4"} -relation2 = dataclasses.replace(relation, remote_app_data={"3", "4"}) +# make a copy of relation, but with remote_app_data set to {"3": "4"} +relation2 = dataclasses.replace(relation, remote_app_data={"3": "4"}) ``` # Consistency checks diff --git a/tests/readme-conftest.py b/tests/readme-conftest.py new file mode 100644 index 00000000..d125b930 --- /dev/null +++ b/tests/readme-conftest.py @@ -0,0 +1,56 @@ +"""pytest configuration for testing the README""" + +import ops + +import scenario + + +def pytest_markdown_docs_globals(): + class MyCharm(ops.CharmBase): + META = {"name": "mycharm", "storage": {"foo": {"type": "filesystem"}}} + + class SimpleCharm(ops.CharmBase): + META = {"name": "simplecharm"} + + def __init__(self, framework: ops.Framework): + super().__init__(framework) + framework.observe(self.on.start, self._on_start) + + def _on_start(self, _: ops.StartEvent): + pass + + class HistoryCharm(ops.CharmBase): + META = {"name": "historycharm"} + + def __init__(self, framework: ops.Framework): + super().__init__(framework) + framework.observe(self.on.start, self._on_start) + + def _on_start(self, _: ops.StartEvent): + self.unit.set_workload_version("1") + self.unit.set_workload_version("1.2") + self.unit.set_workload_version("1.5") + self.unit.set_workload_version("2.0") + + class PortCharm(ops.CharmBase): + META = {"name": "portcharm"} + + def __init__(self, framework: ops.Framework): + super().__init__(framework) + framework.observe(self.on.start, self._on_start) + framework.observe(self.on.stop, self._on_stop) + + def _on_start(self, _: ops.StartEvent): + self.unit.open_port(protocol="tcp", port=42) + + def _on_stop(self, _: ops.StopEvent): + self.unit.close_port(protocol="tcp", port=42) + + return { + "ops": ops, + "scenario": scenario, + "MyCharm": MyCharm, + "HistoryCharm": HistoryCharm, + "PortCharm": PortCharm, + "SimpleCharm": SimpleCharm, + } diff --git a/tox.ini b/tox.ini index 3ec58e80..6ee68690 100644 --- a/tox.ini +++ b/tox.ini @@ -68,3 +68,20 @@ deps = commands = ruff format {[vars]tst_path} {[vars]src_path} isort --profile black {[vars]tst_path} {[vars]src_path} + +[testenv:test-readme] +description = Test code snippets in the README. +skip_install = true +allowlist_externals = + mkdir + cp +deps = + . + ops + pytest + pytest-markdown-docs +commands = + mkdir -p {envtmpdir}/test-readme + cp {toxinidir}/README.md {envtmpdir}/test-readme/README.md + cp {toxinidir}/tests/readme-conftest.py {envtmpdir}/test-readme/conftest.py + pytest -v --tb native --log-cli-level=INFO -s {posargs} --markdown-docs {envtmpdir}/test-readme/README.md From d62a8d7316a4530d5600ef74b070e7fad3b18fff Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 24 Apr 2024 14:31:41 +1200 Subject: [PATCH 17/23] Support 'ctx.on.event_name' for specifying events. --- scenario/context.py | 322 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 322 insertions(+) diff --git a/scenario/context.py b/scenario/context.py index 407281f1..1a7e58fe 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -162,12 +162,72 @@ def _get_output(self): return self._ctx._finalize_action(self._ctx.output_state) # noqa +<<<<<<< HEAD +======= +@dataclasses.dataclass +class _EventSource: + kind: str + + +@dataclasses.dataclass +class _SecretEventSource(_EventSource): + _secret: Optional[Secret] = None + _revision: Optional[int] = None + + def __call__(self, secret: Secret, revision: Optional[int] = None) -> Self: + """Link to a specific scenario.Secret object.""" + self._secret = secret + self._revision = revision + return self + + +@dataclasses.dataclass +class _RelationEventSource(_EventSource): + name: str + _unit_id: Optional[str] = None + _departing_unit_id: Optional[str] = None + + def __call__( + self, + unit: Optional[str] = None, + departing_unit: Optional[str] = None, + ) -> Self: + self._unit_id = unit + self._departing_unit_id = departing_unit + return self + + +@dataclasses.dataclass +class _StorageEventSource(_EventSource): + name: str + + +@dataclasses.dataclass +class _ContainerEventSource(_EventSource): + name: str + + +@dataclasses.dataclass +class _ActionEventSource(_EventSource): + name: str + _action: Optional[Action] = None + + def __call__(self, action: Action) -> Self: + """Provide a scenario.Action object, in order to specify params or id.""" + if action.name != self.name: + raise RuntimeError("WRITE AN ERROR MESSAGE HERE") + self._action = action + return self + + +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) class _CharmEvents: """Events generated by Juju pertaining to application lifecycle. By default, the events listed as attributes of this class will be provided via the :attr:`Context.on` attribute. For example:: +<<<<<<< HEAD ctx.run(ctx.on.config_changed(), state) This behaves similarly to the :class:`ops.CharmEvents` class but is much @@ -307,6 +367,102 @@ def storage_detaching(storage: Storage): @staticmethod def pebble_ready(container: Container): return _Event(f"{container.name}_pebble_ready", container=container) +======= + ctx.run(ctx.on.config_changed, state) + + In addition to the events listed as attributes of this class, + dynamically-named events will also be defined based on the charm's + metadata for relations, storage, actions, and containers. These named + events may be accessed as ``ctx.on._``, for example:: + + ctx.run(ctx.on.workload_pebble_ready, state) + + See also the :class:`ops.CharmEvents` class. + """ + + # TODO: There are lots of suffix definitions in state - we should be able to re-use those. + install = _EventSource("install") + start = _EventSource("start") + stop = _EventSource("stop") + remove = _EventSource("remove") + update_status = _EventSource("update_status") + config_changed = _EventSource("config_changed") + upgrade_charm = _EventSource("upgrade_charm") + pre_series_upgrade = _EventSource("pre_series_upgrade") + post_series_upgrade = _EventSource("post_series_upgrade") + leader_elected = _EventSource("leader_elected") + secret_changed = _SecretEventSource("secret_changed") + secret_expired = _SecretEventSource("secret_expired") + secret_rotate = _SecretEventSource("secret_rotate") + secret_remove = _SecretEventSource("secret_remove") + collect_app_status = _EventSource("collect_app_status") + collect_unit_status = _EventSource("collect_unit_status") + + def __init__(self, charm_spec: _CharmSpec): + for relation_name, _ in charm_spec.get_all_relations(): + relation_name = relation_name.replace("-", "_") + setattr( + self, + f"{relation_name}_relation_created", + _RelationEventSource("relation_created", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_joined", + _RelationEventSource("relation_joined", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_changed", + _RelationEventSource("relation_changed", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_departed", + _RelationEventSource("relation_departed", relation_name), + ) + setattr( + self, + f"{relation_name}_relation_broken", + _RelationEventSource("relation_broken", relation_name), + ) + + for storage_name in charm_spec.meta.get("storage", ()): + storage_name = storage_name.replace("-", "_") + setattr( + self, + f"{storage_name}_storage_attached", + _StorageEventSource("storage_attached", storage_name), + ) + setattr( + self, + f"{storage_name}_storage_detaching", + _StorageEventSource("storage_detaching", storage_name), + ) + + for action_name in charm_spec.actions or (): + action_name = action_name.replace("-", "_") + setattr( + self, + f"{action_name}_action", + _ActionEventSource("action", action_name), + ) + + for container_name in charm_spec.meta.get("containers", ()): + container_name = container_name.replace("-", "_") + setattr( + self, + f"{container_name}_pebble_ready", + _ContainerEventSource("pebble_ready", container_name), + ) + + +# setattr( +# self, +# f"{container_name}_pebble_custom_notice", +# _ContainerEventSource("pebble_custom_notice", container_name), +# ) +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) class Context: @@ -446,7 +602,11 @@ def __init__( self._action_results: Optional[Dict[str, str]] = None self._action_failure: Optional[str] = None +<<<<<<< HEAD self.on = _CharmEvents() +======= + self.on = _CharmEvents(self.charm_spec) +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) def _set_output_state(self, output_state: "State"): """Hook for Runtime to set the output state.""" @@ -482,7 +642,124 @@ def _record_status(self, state: "State", is_app: bool): else: self.unit_status_history.append(cast("_EntityStatus", state.unit_status)) +<<<<<<< HEAD def manager(self, event: "_Event", state: "State"): +======= + @staticmethod + def _coalesce_action(action: Union[str, Action, _ActionEventSource]) -> Action: + """Validate the action argument and cast to Action.""" + if isinstance(action, str): + return Action(action) + + if isinstance(action, _ActionEventSource): + if action._action: + return action._action + return Action(action.name) + + if not isinstance(action, Action): + raise InvalidActionError( + f"Expected Action or action name; got {type(action)}", + ) + return action + + # TODO: These don't really need to be List, probably Iterable is fine? Really ought to be Mapping ... + @staticmethod + def _coalesce_event( + event: Union[str, Event, _EventSource], + *, + containers: Optional[List[Container]] = None, + storages: Optional[List[Storage]] = None, + relations: Optional[List["AnyRelation"]] = None, + ) -> Event: + """Validate the event argument and cast to Event.""" + if isinstance(event, str): + event = Event(event) + + if isinstance(event, _EventSource): + if event.kind == "action": + raise InvalidEventError( + "Cannot Context.run() action events. " + "Use Context.run_action instead.", + ) + if isinstance(event, _SecretEventSource): + if (secret := event._secret) is None: + raise InvalidEventError( + "A secret must be provided, for example: " + "ctx.run(ctx.on.secret_changed(secret=secret), state)", + ) + else: + secret = None + secret_revision = getattr(event, "_revision", None) + # TODO: These can probably do Event.bind() + if isinstance(event, _StorageEventSource): + # TODO: It would be great if this was a mapping, not a list. + for storage in storages or (): + if storage.name == event.name: + break + else: + raise InvalidEventError( + f"Attempting to run {event.name}_{event.kind}, but " + f"{event.name} is not a storage in the state.", + ) + else: + storage = None + if isinstance(event, _ContainerEventSource): + # TODO: It would be great if this was a mapping, not a list. + for container in containers or (): + if container.name == event.name: + break + else: + raise InvalidEventError( + f"Attempting to run {event.name}_{event.kind}, but " + f"{event.name} is not a container in the state.", + ) + else: + container = None + if isinstance(event, _RelationEventSource): + # TODO: It would be great if this was a mapping, not a list. + for relation in relations or (): + if relation.endpoint == event.name: + break + else: + raise InvalidEventError( + f"Attempting to run {event.name}_{event.kind}, but " + f"{event.name} is not a relation in the state.", + ) + else: + relation = None + relation_remote_unit_id = getattr(event, "_unit_id", None) + relation_departed_unit_id = getattr(event, "_departing_unit_id", None) + if hasattr(event, "name"): + path = f"{event.name}_{event.kind}" # type: ignore + else: + path = event.kind + event = Event( + path, + secret=secret, + secret_revision=secret_revision, + storage=storage, + container=container, + relation=relation, + relation_remote_unit_id=relation_remote_unit_id, + relation_departed_unit_id=relation_departed_unit_id, + ) + + if not isinstance(event, Event): + raise InvalidEventError(f"Expected Event | str, got {type(event)}") + + if event._is_action_event: # noqa + raise InvalidEventError( + "Cannot Context.run() action events. " + "Use Context.run_action instead.", + ) + return event + + def manager( + self, + event: Union["Event", str], + state: "State", + ): +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) """Context manager to introspect live charm object before and after the event is emitted. Usage: @@ -513,17 +790,44 @@ def action_manager(self, action: "Action", state: "State"): return _ActionManager(self, action, state) @contextmanager +<<<<<<< HEAD def _run_event(self, event: "_Event", state: "State"): with self._run(event=event, state=state) as ops: yield ops def run(self, event: "_Event", state: "State") -> "State": +======= + def _run_event( + self, + event: Union["_EventSource", "Event", str], + state: "State", + ): + _event = self._coalesce_event( + event, + containers=state.containers, + storages=state.storage, + relations=state.relations, + ) + with self._run(event=_event, state=state) as ops: + yield ops + + def run( + self, + event: Union["_EventSource", "Event", str], + state: "State", + ) -> "State": +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) """Trigger a charm execution with an Event and a State. Calling this function will call ``ops.main`` and set up the context according to the specified ``State``, then emit the event on the charm. +<<<<<<< HEAD :arg event: the Event that the charm will respond to. +======= + :arg event: the Event that the charm will respond to. Can be a string or an Event instance + or an _EventSource. +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Event. """ @@ -533,7 +837,15 @@ def run(self, event: "_Event", state: "State") -> "State": ops.emit() return self.output_state +<<<<<<< HEAD def run_action(self, action: "Action", state: "State") -> ActionOutput: +======= + def run_action( + self, + action: Union["_ActionEventSource", "Action", str], + state: "State", + ) -> ActionOutput: +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) """Trigger a charm execution with an Action and a State. Calling this function will call ``ops.main`` and set up the context according to the @@ -563,8 +875,18 @@ def _finalize_action(self, state_out: "State"): return ao @contextmanager +<<<<<<< HEAD def _run_action(self, action: "Action", state: "State"): with self._run(event=action.event, state=state) as ops: +======= + def _run_action( + self, + action: Union["_ActionEventSource", "Action", str], + state: "State", + ): + _action = self._coalesce_action(action) + with self._run(event=_action.event, state=state) as ops: +>>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) yield ops @contextmanager From adb56226cf36aa5536c3b28227b41238402c4922 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 4 Jun 2024 16:44:06 +1200 Subject: [PATCH 18/23] Update tests and docs to match final (hopefully\!) API decision. --- README.md | 2 +- tests/test_context_on.py | 23 +++++++++++++++-------- tests/test_e2e/test_event.py | 4 +--- tests/test_plugin.py | 6 +++--- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index fad09daa..1efa8249 100644 --- a/README.md +++ b/README.md @@ -744,7 +744,7 @@ Since `ops 2.6.0`, charms can invoke the `open-port`, `close-port`, and `opened- - simulate a charm run with a port opened by some previous execution ctx = scenario.Context(MyCharm, meta=MyCharm.META) -ctx.run("start", scenario.State(opened_ports=[scenario.Port("tcp", 42)])) +ctx.run(ctx.on.start(), scenario.State(opened_ports=[scenario.Port("tcp", 42)])) ``` - assert that a charm has called `open-port` or `close-port`: ```python diff --git a/tests/test_context_on.py b/tests/test_context_on.py index be8c70b5..1ad35b37 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -149,10 +149,10 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(event_name, event_kind): +def test_storage_events(as_kwarg, storage_index, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storage = scenario.Storage("foo") - state_in = scenario.State(storage=[storage]) + storages = [scenario.Storage("foo", index) for index in range((storage_index or 0) + 1)] + state_in = scenario.State(storage=storages) # These look like: # ctx.run(ctx.on.storage_attached(storage), state) with ctx.manager(getattr(ctx.on, event_name)(storage), state_in) as mgr: @@ -161,8 +161,17 @@ def test_storage_events(event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storage.name - assert event.storage.index == storage.index + assert event.storage.name == storages[-1].name + assert event.storage.index == storages[-1].index + + +@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) +def test_storage_events_as_positional_arg(event_name): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) + with pytest.assertRaises(ValueError): + ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) def test_action_event_no_params(): @@ -305,9 +314,7 @@ def test_relation_unit_events(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) - with ctx.manager( - getattr(ctx.on, event_name)(relation, remote_unit=2), state_in - ) as mgr: + with ctx.manager(getattr(ctx.on, event_name)(relation, unit=2), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index fde6badc..b7ba49c8 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -84,9 +84,7 @@ def _foo(self, e): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run( - ctx.on.start(), State(deferred=[_Event("update-status").deferred(MyCharm._foo)]) - ) + ctx.run(ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)])) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 38bca584..b802b9eb 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -12,14 +12,14 @@ def test_plugin_ctx_run(pytester): from scenario import State from scenario import Context import ops - + class MyCharm(ops.CharmBase): pass - + @pytest.fixture def context(): return Context(charm_type=MyCharm, meta={"name": "foo"}) - + def test_sth(context): context.run(context.on.start(), State()) """ From fcff42fbe4a430fe16336b2cab71e3290b87aef2 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 10:51:42 +1200 Subject: [PATCH 19/23] Fix tests. The failing test are (a) ones that need to be rewritten for the new system, (b) ones to do with custom events. --- scenario/context.py | 322 --------------------------------------- scenario/state.py | 2 - tests/test_context_on.py | 23 +-- 3 files changed, 8 insertions(+), 339 deletions(-) diff --git a/scenario/context.py b/scenario/context.py index 1a7e58fe..407281f1 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -162,72 +162,12 @@ def _get_output(self): return self._ctx._finalize_action(self._ctx.output_state) # noqa -<<<<<<< HEAD -======= -@dataclasses.dataclass -class _EventSource: - kind: str - - -@dataclasses.dataclass -class _SecretEventSource(_EventSource): - _secret: Optional[Secret] = None - _revision: Optional[int] = None - - def __call__(self, secret: Secret, revision: Optional[int] = None) -> Self: - """Link to a specific scenario.Secret object.""" - self._secret = secret - self._revision = revision - return self - - -@dataclasses.dataclass -class _RelationEventSource(_EventSource): - name: str - _unit_id: Optional[str] = None - _departing_unit_id: Optional[str] = None - - def __call__( - self, - unit: Optional[str] = None, - departing_unit: Optional[str] = None, - ) -> Self: - self._unit_id = unit - self._departing_unit_id = departing_unit - return self - - -@dataclasses.dataclass -class _StorageEventSource(_EventSource): - name: str - - -@dataclasses.dataclass -class _ContainerEventSource(_EventSource): - name: str - - -@dataclasses.dataclass -class _ActionEventSource(_EventSource): - name: str - _action: Optional[Action] = None - - def __call__(self, action: Action) -> Self: - """Provide a scenario.Action object, in order to specify params or id.""" - if action.name != self.name: - raise RuntimeError("WRITE AN ERROR MESSAGE HERE") - self._action = action - return self - - ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) class _CharmEvents: """Events generated by Juju pertaining to application lifecycle. By default, the events listed as attributes of this class will be provided via the :attr:`Context.on` attribute. For example:: -<<<<<<< HEAD ctx.run(ctx.on.config_changed(), state) This behaves similarly to the :class:`ops.CharmEvents` class but is much @@ -367,102 +307,6 @@ def storage_detaching(storage: Storage): @staticmethod def pebble_ready(container: Container): return _Event(f"{container.name}_pebble_ready", container=container) -======= - ctx.run(ctx.on.config_changed, state) - - In addition to the events listed as attributes of this class, - dynamically-named events will also be defined based on the charm's - metadata for relations, storage, actions, and containers. These named - events may be accessed as ``ctx.on._``, for example:: - - ctx.run(ctx.on.workload_pebble_ready, state) - - See also the :class:`ops.CharmEvents` class. - """ - - # TODO: There are lots of suffix definitions in state - we should be able to re-use those. - install = _EventSource("install") - start = _EventSource("start") - stop = _EventSource("stop") - remove = _EventSource("remove") - update_status = _EventSource("update_status") - config_changed = _EventSource("config_changed") - upgrade_charm = _EventSource("upgrade_charm") - pre_series_upgrade = _EventSource("pre_series_upgrade") - post_series_upgrade = _EventSource("post_series_upgrade") - leader_elected = _EventSource("leader_elected") - secret_changed = _SecretEventSource("secret_changed") - secret_expired = _SecretEventSource("secret_expired") - secret_rotate = _SecretEventSource("secret_rotate") - secret_remove = _SecretEventSource("secret_remove") - collect_app_status = _EventSource("collect_app_status") - collect_unit_status = _EventSource("collect_unit_status") - - def __init__(self, charm_spec: _CharmSpec): - for relation_name, _ in charm_spec.get_all_relations(): - relation_name = relation_name.replace("-", "_") - setattr( - self, - f"{relation_name}_relation_created", - _RelationEventSource("relation_created", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_joined", - _RelationEventSource("relation_joined", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_changed", - _RelationEventSource("relation_changed", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_departed", - _RelationEventSource("relation_departed", relation_name), - ) - setattr( - self, - f"{relation_name}_relation_broken", - _RelationEventSource("relation_broken", relation_name), - ) - - for storage_name in charm_spec.meta.get("storage", ()): - storage_name = storage_name.replace("-", "_") - setattr( - self, - f"{storage_name}_storage_attached", - _StorageEventSource("storage_attached", storage_name), - ) - setattr( - self, - f"{storage_name}_storage_detaching", - _StorageEventSource("storage_detaching", storage_name), - ) - - for action_name in charm_spec.actions or (): - action_name = action_name.replace("-", "_") - setattr( - self, - f"{action_name}_action", - _ActionEventSource("action", action_name), - ) - - for container_name in charm_spec.meta.get("containers", ()): - container_name = container_name.replace("-", "_") - setattr( - self, - f"{container_name}_pebble_ready", - _ContainerEventSource("pebble_ready", container_name), - ) - - -# setattr( -# self, -# f"{container_name}_pebble_custom_notice", -# _ContainerEventSource("pebble_custom_notice", container_name), -# ) ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) class Context: @@ -602,11 +446,7 @@ def __init__( self._action_results: Optional[Dict[str, str]] = None self._action_failure: Optional[str] = None -<<<<<<< HEAD self.on = _CharmEvents() -======= - self.on = _CharmEvents(self.charm_spec) ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) def _set_output_state(self, output_state: "State"): """Hook for Runtime to set the output state.""" @@ -642,124 +482,7 @@ def _record_status(self, state: "State", is_app: bool): else: self.unit_status_history.append(cast("_EntityStatus", state.unit_status)) -<<<<<<< HEAD def manager(self, event: "_Event", state: "State"): -======= - @staticmethod - def _coalesce_action(action: Union[str, Action, _ActionEventSource]) -> Action: - """Validate the action argument and cast to Action.""" - if isinstance(action, str): - return Action(action) - - if isinstance(action, _ActionEventSource): - if action._action: - return action._action - return Action(action.name) - - if not isinstance(action, Action): - raise InvalidActionError( - f"Expected Action or action name; got {type(action)}", - ) - return action - - # TODO: These don't really need to be List, probably Iterable is fine? Really ought to be Mapping ... - @staticmethod - def _coalesce_event( - event: Union[str, Event, _EventSource], - *, - containers: Optional[List[Container]] = None, - storages: Optional[List[Storage]] = None, - relations: Optional[List["AnyRelation"]] = None, - ) -> Event: - """Validate the event argument and cast to Event.""" - if isinstance(event, str): - event = Event(event) - - if isinstance(event, _EventSource): - if event.kind == "action": - raise InvalidEventError( - "Cannot Context.run() action events. " - "Use Context.run_action instead.", - ) - if isinstance(event, _SecretEventSource): - if (secret := event._secret) is None: - raise InvalidEventError( - "A secret must be provided, for example: " - "ctx.run(ctx.on.secret_changed(secret=secret), state)", - ) - else: - secret = None - secret_revision = getattr(event, "_revision", None) - # TODO: These can probably do Event.bind() - if isinstance(event, _StorageEventSource): - # TODO: It would be great if this was a mapping, not a list. - for storage in storages or (): - if storage.name == event.name: - break - else: - raise InvalidEventError( - f"Attempting to run {event.name}_{event.kind}, but " - f"{event.name} is not a storage in the state.", - ) - else: - storage = None - if isinstance(event, _ContainerEventSource): - # TODO: It would be great if this was a mapping, not a list. - for container in containers or (): - if container.name == event.name: - break - else: - raise InvalidEventError( - f"Attempting to run {event.name}_{event.kind}, but " - f"{event.name} is not a container in the state.", - ) - else: - container = None - if isinstance(event, _RelationEventSource): - # TODO: It would be great if this was a mapping, not a list. - for relation in relations or (): - if relation.endpoint == event.name: - break - else: - raise InvalidEventError( - f"Attempting to run {event.name}_{event.kind}, but " - f"{event.name} is not a relation in the state.", - ) - else: - relation = None - relation_remote_unit_id = getattr(event, "_unit_id", None) - relation_departed_unit_id = getattr(event, "_departing_unit_id", None) - if hasattr(event, "name"): - path = f"{event.name}_{event.kind}" # type: ignore - else: - path = event.kind - event = Event( - path, - secret=secret, - secret_revision=secret_revision, - storage=storage, - container=container, - relation=relation, - relation_remote_unit_id=relation_remote_unit_id, - relation_departed_unit_id=relation_departed_unit_id, - ) - - if not isinstance(event, Event): - raise InvalidEventError(f"Expected Event | str, got {type(event)}") - - if event._is_action_event: # noqa - raise InvalidEventError( - "Cannot Context.run() action events. " - "Use Context.run_action instead.", - ) - return event - - def manager( - self, - event: Union["Event", str], - state: "State", - ): ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) """Context manager to introspect live charm object before and after the event is emitted. Usage: @@ -790,44 +513,17 @@ def action_manager(self, action: "Action", state: "State"): return _ActionManager(self, action, state) @contextmanager -<<<<<<< HEAD def _run_event(self, event: "_Event", state: "State"): with self._run(event=event, state=state) as ops: yield ops def run(self, event: "_Event", state: "State") -> "State": -======= - def _run_event( - self, - event: Union["_EventSource", "Event", str], - state: "State", - ): - _event = self._coalesce_event( - event, - containers=state.containers, - storages=state.storage, - relations=state.relations, - ) - with self._run(event=_event, state=state) as ops: - yield ops - - def run( - self, - event: Union["_EventSource", "Event", str], - state: "State", - ) -> "State": ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) """Trigger a charm execution with an Event and a State. Calling this function will call ``ops.main`` and set up the context according to the specified ``State``, then emit the event on the charm. -<<<<<<< HEAD :arg event: the Event that the charm will respond to. -======= - :arg event: the Event that the charm will respond to. Can be a string or an Event instance - or an _EventSource. ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) :arg state: the State instance to use as data source for the hook tool calls that the charm will invoke when handling the Event. """ @@ -837,15 +533,7 @@ def run( ops.emit() return self.output_state -<<<<<<< HEAD def run_action(self, action: "Action", state: "State") -> ActionOutput: -======= - def run_action( - self, - action: Union["_ActionEventSource", "Action", str], - state: "State", - ) -> ActionOutput: ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) """Trigger a charm execution with an Action and a State. Calling this function will call ``ops.main`` and set up the context according to the @@ -875,18 +563,8 @@ def _finalize_action(self, state_out: "State"): return ao @contextmanager -<<<<<<< HEAD def _run_action(self, action: "Action", state: "State"): with self._run(event=action.event, state=state) as ops: -======= - def _run_action( - self, - action: Union["_ActionEventSource", "Action", str], - state: "State", - ): - _action = self._coalesce_action(action) - with self._run(event=_action.event, state=state) as ops: ->>>>>>> d51ea25 (Support 'ctx.on.event_name' for specifying events.) yield ops @contextmanager diff --git a/scenario/state.py b/scenario/state.py index ae7d739b..b6884b4a 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -578,7 +578,6 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated pebble filesystem in this context.""" return ctx._get_container_root(self.name) - _RawStatusLiteral = Literal[ "waiting", "blocked", @@ -704,7 +703,6 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated filesystem root in this context.""" return ctx._get_storage_root(self.name, self.index) - @dataclasses.dataclass(frozen=True) class State: """Represents the juju-owned portion of a unit's state. diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 1ad35b37..be8c70b5 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -149,10 +149,10 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(as_kwarg, storage_index, event_name, event_kind): +def test_storage_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storages = [scenario.Storage("foo", index) for index in range((storage_index or 0) + 1)] - state_in = scenario.State(storage=storages) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) # These look like: # ctx.run(ctx.on.storage_attached(storage), state) with ctx.manager(getattr(ctx.on, event_name)(storage), state_in) as mgr: @@ -161,17 +161,8 @@ def test_storage_events(as_kwarg, storage_index, event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storages[-1].name - assert event.storage.index == storages[-1].index - - -@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) -def test_storage_events_as_positional_arg(event_name): - ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storage = scenario.Storage("foo") - state_in = scenario.State(storage=[storage]) - with pytest.assertRaises(ValueError): - ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) + assert event.storage.name == storage.name + assert event.storage.index == storage.index def test_action_event_no_params(): @@ -314,7 +305,9 @@ def test_relation_unit_events(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) - with ctx.manager(getattr(ctx.on, event_name)(relation, unit=2), state_in) as mgr: + with ctx.manager( + getattr(ctx.on, event_name)(relation, remote_unit=2), state_in + ) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) From 9d56f640a27f0903c1325d5a031800f3fe645e5a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 18:38:12 +1200 Subject: [PATCH 20/23] Various README updates. --- README.md | 50 +++++++++++++++++++++++--------------------------- tox.ini | 2 +- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 9431d61c..d405a648 100644 --- a/README.md +++ b/README.md @@ -213,7 +213,7 @@ hook execution: ```python # ... ctx = scenario.Context(HistoryCharm, meta={"name": "foo"}) -ctx.run("start", scenario.State()) +ctx.run(ctx.on.start(), scenario.State()) assert ctx.workload_version_history == ['1', '1.2', '1.5'] # ... ``` @@ -424,21 +424,16 @@ relation.remote_unit_name # "zookeeper/42" ### Triggering Relation Events -If you want to trigger relation events, the easiest way to do so is get a hold of the Relation instance and grab the -event from one of its aptly-named properties: +If you want to trigger relation events, use `ctx.on.relation_changed` (and so +on for the other relation events) and pass the relation object: ```python -relation = scenario.Relation(endpoint="foo", interface="bar") -changed_event = relation.changed_event -joined_event = relation.joined_event -# ... -``` - -This is in fact syntactic sugar for: +ctx = scenario.Context(MyCharm, meta=MyCharm.META) -```python relation = scenario.Relation(endpoint="foo", interface="bar") -changed_event = scenario.Event('foo-relation-changed', relation=relation) +changed_event = ctx.on.relation_changed(relation=relation) +joined_event = ctx.on.relation_joined(relation=relation) +# ... ``` The reason for this construction is that the event is associated with some relation-specific metadata, that Scenario @@ -476,20 +471,16 @@ All relation events have some additional metadata that does not belong in the Re relation-joined event, the name of the (remote) unit that is joining the relation. That is what determines what `ops.model.Unit` you get when you get `RelationJoinedEvent().unit` in an event handler. -In order to supply this parameter, you will have to **call** the event object and pass as `remote_unit_id` the id of the +In order to supply this parameter, as well as the relation object, pass as `remote_unit` the id of the remote unit that the event is about. The reason that this parameter is not supplied to `Relation` but to relation events, is that the relation already ties 'this app' to some 'remote app' (cfr. the `Relation.remote_app_name` attr), but not to a specific unit. What remote unit this event is about is not a `State` concern but an `Event` one. -The `remote_unit_id` will default to the first ID found in the relation's `remote_units_data`, but if the test you are -writing is close to that domain, you should probably override it and pass it manually. - ```python -relation = scenario.Relation(endpoint="foo", interface="bar") -remote_unit_2_is_joining_event = relation.joined_event(remote_unit_id=2) +ctx = scenario.Context(MyCharm, meta=MyCharm.META) -# which is syntactic sugar for: -remote_unit_2_is_joining_event = scenario.Event('foo-relation-changed', relation=relation, relation_remote_unit_id=2) +relation = scenario.Relation(endpoint="foo", interface="bar") +remote_unit_2_is_joining_event = ctx.on.relation_joined(relation, remote_unit=2) ``` ## Networks @@ -686,7 +677,7 @@ storage = scenario.Storage("foo") # Setup storage with some content: (storage.get_filesystem(ctx) / "myfile.txt").write_text("helloworld") -with ctx.manager("update-status", scenario.State(storage=[storage])) as mgr: +with ctx.manager(ctx.on.update_status(), scenario.State(storage=[storage])) as mgr: foo = mgr.charm.model.storages["foo"][0] loc = foo.location path = loc / "myfile.txt" @@ -857,7 +848,7 @@ So, the only consistency-level check we enforce in Scenario when it comes to res import pathlib ctx = scenario.Context(MyCharm, meta={'name': 'juliette', "resources": {"foo": {"type": "oci-image"}}}) -with ctx.manager("start", scenario.State(resources={'foo': '/path/to/resource.tar'})) as mgr: +with ctx.manager(ctx.on.start(), scenario.State(resources={'foo': '/path/to/resource.tar'})) as mgr: # If the charm, at runtime, were to call self.model.resources.fetch("foo"), it would get '/path/to/resource.tar' back. path = mgr.charm.model.resources.fetch('foo') assert path == pathlib.Path('/path/to/resource.tar') @@ -959,8 +950,10 @@ You can also generate the 'deferred' data structure (called a DeferredEvent) fro handler): ```python continuation -deferred_start = scenario.Event('start').deferred(MyCharm._on_start) -deferred_install = scenario.Event('install').deferred(MyCharm._on_start) +ctx = scenario.Context(MyCharm, meta={"name": "deferring"}) + +deferred_start = ctx.on.start().deferred(MyCharm._on_start) +deferred_install = ctx.on.install().deferred(MyCharm._on_start) ``` On the output side, you can verify that an event that you expect to have been deferred during this trigger, has indeed @@ -1009,8 +1002,10 @@ def test_start_on_deferred_update_status(MyCharm): but you can also use a shortcut from the relation event itself: ```python continuation +ctx = scenario.Context(MyCharm, meta={"name": "deferring"}) + foo_relation = scenario.Relation('foo') -foo_relation.changed_event.deferred(handler=MyCharm._on_foo_relation_changed) +deferred_event = ctx.on.relation_changed(foo_relation).deferred(handler=MyCharm._on_foo_relation_changed) ``` =# Live charm introspection @@ -1095,11 +1090,12 @@ class MyCharmType(ops.CharmBase): td = tempfile.TemporaryDirectory() -state = scenario.Context( +ctx = scenario.Context( charm_type=MyCharmType, meta={'name': 'my-charm-name'}, charm_root=td.name -).run(ctx.on.start(), scenario.State()) +) +state = ctx.run(ctx.on.start(), scenario.State()) ``` Do this, and you will be able to set up said directory as you like before the charm is run, as well as verify its diff --git a/tox.ini b/tox.ini index 6ee68690..6fb2c2f1 100644 --- a/tox.ini +++ b/tox.ini @@ -76,7 +76,7 @@ allowlist_externals = mkdir cp deps = - . + -e . ops pytest pytest-markdown-docs From 66b53102199587ad79dd3e3a354b25fcea4b43ac Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 18:39:14 +1200 Subject: [PATCH 21/23] Typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d405a648..567f2210 100644 --- a/README.md +++ b/README.md @@ -1008,7 +1008,7 @@ foo_relation = scenario.Relation('foo') deferred_event = ctx.on.relation_changed(foo_relation).deferred(handler=MyCharm._on_foo_relation_changed) ``` -=# Live charm introspection +# Live charm introspection Scenario is a black-box, state-transition testing framework. It makes it trivial to assert that a status went from A to B, but not to assert that, in the context of this charm execution, with this state, a certain charm-internal method was called and returned a From 72ab734b432550f1ecaf6c553a1962764ad76008 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 18:47:14 +1200 Subject: [PATCH 22/23] Style fixes. --- scenario/state.py | 2 ++ tests/test_e2e/test_event.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/scenario/state.py b/scenario/state.py index b6884b4a..ae7d739b 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -578,6 +578,7 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated pebble filesystem in this context.""" return ctx._get_container_root(self.name) + _RawStatusLiteral = Literal[ "waiting", "blocked", @@ -703,6 +704,7 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated filesystem root in this context.""" return ctx._get_storage_root(self.name, self.index) + @dataclasses.dataclass(frozen=True) class State: """Represents the juju-owned portion of a unit's state. diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index b7ba49c8..fde6badc 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -84,7 +84,9 @@ def _foo(self, e): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run(ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)])) + ctx.run( + ctx.on.start(), State(deferred=[_Event("update-status").deferred(MyCharm._foo)]) + ) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ From 550136b8b0280276a4c4dcbc51c95ffe32483db8 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 18:49:05 +1200 Subject: [PATCH 23/23] Align with upstream. --- scenario/sequences.py | 140 ------------------------------------------ 1 file changed, 140 deletions(-) delete mode 100644 scenario/sequences.py diff --git a/scenario/sequences.py b/scenario/sequences.py deleted file mode 100644 index 5ebb7601..00000000 --- a/scenario/sequences.py +++ /dev/null @@ -1,140 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. -import copy -import dataclasses -import typing -from itertools import chain -from typing import Any, Callable, Dict, Iterable, Optional, TextIO, Type, Union - -from scenario import Context -from scenario.logger import logger as scenario_logger -from scenario.state import ( - ATTACH_ALL_STORAGES, - BREAK_ALL_RELATIONS, - CREATE_ALL_RELATIONS, - DETACH_ALL_STORAGES, - State, - _Event, -) - -if typing.TYPE_CHECKING: # pragma: no cover - from ops.testing import CharmType - -CharmMeta = Optional[Union[str, TextIO, dict]] -logger = scenario_logger.getChild("scenario") - - -def decompose_meta_event(meta_event: _Event, state: State): - # decompose the meta event - - if meta_event.name in [ATTACH_ALL_STORAGES, DETACH_ALL_STORAGES]: - logger.warning(f"meta-event {meta_event.name} not supported yet") - return - - is_rel_created_meta_event = meta_event.name == CREATE_ALL_RELATIONS - is_rel_broken_meta_event = meta_event.name == BREAK_ALL_RELATIONS - if is_rel_broken_meta_event: - for relation in state.relations: - event = relation.broken_event - logger.debug(f"decomposed meta {meta_event.name}: {event}") - yield event, copy.deepcopy(state) - elif is_rel_created_meta_event: - for relation in state.relations: - event = relation.created_event - logger.debug(f"decomposed meta {meta_event.name}: {event}") - yield event, copy.deepcopy(state) - else: - raise RuntimeError(f"unknown meta-event {meta_event.name}") - - -def generate_startup_sequence(state_template: State): - yield from chain( - decompose_meta_event( - _Event(ATTACH_ALL_STORAGES), - copy.deepcopy(state_template), - ), - ((_Event("start"), copy.deepcopy(state_template)),), - decompose_meta_event( - _Event(CREATE_ALL_RELATIONS), - copy.deepcopy(state_template), - ), - ( - ( - _Event( - ( - "leader_elected" - if state_template.leader - else "leader_settings_changed" - ), - ), - copy.deepcopy(state_template), - ), - (_Event("config_changed"), copy.deepcopy(state_template)), - (_Event("install"), copy.deepcopy(state_template)), - ), - ) - - -def generate_teardown_sequence(state_template: State): - yield from chain( - decompose_meta_event( - _Event(BREAK_ALL_RELATIONS), - copy.deepcopy(state_template), - ), - decompose_meta_event( - _Event(DETACH_ALL_STORAGES), - copy.deepcopy(state_template), - ), - ( - (_Event("stop"), copy.deepcopy(state_template)), - (_Event("remove"), copy.deepcopy(state_template)), - ), - ) - - -def generate_builtin_sequences(template_states: Iterable[State]): - for template_state in template_states: - yield from chain( - generate_startup_sequence(template_state), - generate_teardown_sequence(template_state), - ) - - -def check_builtin_sequences( - charm_type: Type["CharmType"], - meta: Optional[Dict[str, Any]] = None, - actions: Optional[Dict[str, Any]] = None, - config: Optional[Dict[str, Any]] = None, - template_state: State = None, - pre_event: Optional[Callable[["CharmType"], None]] = None, - post_event: Optional[Callable[["CharmType"], None]] = None, -) -> object: - """Test that all the builtin startup and teardown events can fire without errors. - - This will play both scenarios with and without leadership, and raise any exceptions. - - This is a baseline check that in principle all charms (except specific use-cases perhaps), - should pass out of the box. - - If you want to, you can inject more stringent state checks using the - pre_event and post_event hooks. - """ - - template = template_state if template_state else State() - out = [] - - for event, state in generate_builtin_sequences( - ( - dataclasses.replace(template, leader=True), - dataclasses.replace(template, leader=False), - ), - ): - ctx = Context(charm_type=charm_type, meta=meta, actions=actions, config=config) - with ctx.manager(event, state=state) as mgr: - if pre_event: - pre_event(mgr.charm) - out.append(mgr.run()) - if post_event: - post_event(mgr.charm) - return out