diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b83e0b33..b8809229 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,11 +29,15 @@ repos: rev: 5.12.0 hooks: - id: isort - - repo: https://github.com/psf/black - rev: 23.3.0 + # Run the Ruff linter. + - repo: https://github.com/astral-sh/ruff-pre-commit + # Ruff version. + rev: v0.1.2 hooks: - - id: black - args: [--safe] + # Run the Ruff linter. + - id: ruff + # Run the Ruff formatter. + - id: ruff-format - repo: https://github.com/asottile/blacken-docs rev: 1.13.0 hooks: @@ -48,19 +52,6 @@ repos: hooks: - id: tox-ini-fmt args: ["-p", "fix"] - - repo: https://github.com/PyCQA/flake8 - rev: 6.0.0 - hooks: - - id: flake8 - additional_dependencies: - - flake8-bugbear==23.3.23 - - flake8-comprehensions==3.12 - - flake8-pytest-style==1.7.2 - - flake8-spellcheck==0.28 - - flake8-unused-arguments==0.0.13 - - flake8-noqa==1.3.1 - - pep8-naming==0.13.3 - - flake8-pyproject==1.2.3 - repo: https://github.com/pre-commit/mirrors-prettier rev: "v2.7.1" hooks: diff --git a/README.md b/README.md index 43aa0e00..674d038f 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ available. The charm has no config, no relations, no networks, no leadership, an With that, we can write the simplest possible scenario test: ```python -from scenario import State, Context +from scenario import State, Context, Event from ops.charm import CharmBase from ops.model import UnknownStatus @@ -93,7 +93,7 @@ class MyCharm(CharmBase): def test_scenario_base(): ctx = Context(MyCharm, meta={"name": "foo"}) - out = ctx.run('start', State()) + out = ctx.run(Event("start"), State()) assert out.unit_status == UnknownStatus() ``` @@ -1051,6 +1051,23 @@ state = State(stored_state=[ And the charm's runtime will see `self.stored_State.foo` and `.baz` as expected. Also, you can run assertions on it on the output side the same as any other bit of state. +# Resources + +If your charm requires access to resources, you can make them available to it through `State.resources`. +From the perspective of a 'real' deployed charm, if your charm _has_ resources defined in `metadata.yaml`, they _must_ be made available to the charm. That is a Juju-enforced constraint: you can't deploy a charm without attaching all resources it needs to it. +However, when testing, this constraint is unnecessarily strict (and it would also mean the great majority of all existing tests would break) since a charm will only notice that a resource is not available when it explicitly asks for it, which not many charms do. + +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 metadata. + +```python +from scenario import State, Context +ctx = Context(MyCharm, meta={'name': 'juliette', "resources": {"foo": {"type": "oci-image"}}}) +with ctx.manager("start", 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' +``` + # Emitting custom events While the main use case of Scenario is to emit juju events, i.e. the built-in `start`, `install`, `*-relation-changed`, diff --git a/pyproject.toml b/pyproject.toml index ba555924..b728b128 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "ops-scenario" -version = "5.4.1" +version = "5.5" authors = [ { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" } @@ -50,10 +50,38 @@ scenario = "scenario" include = '\.pyi?$' -[tool.flake8] -dictionaries = ["en_US","python","technical","django"] -max-line-length = 100 -ignore = ["SC100", "SC200", "B008"] +[tool.ruff] +# Same as Black. +line-length = 88 +indent-width = 4 + +# Assume Python 3.11 +target-version = "py311" + +[tool.ruff.lint] +# Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`) codes by default. +select = ["E4", "E7", "E9", "F"] +ignore = [] + +# Allow fix for all enabled rules (when `--fix`) is provided. +fixable = ["ALL"] +unfixable = [] + +# Allow unused variables when underscore-prefixed. +dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" + +[tool.ruff.format] +# Like Black, use double quotes for strings. +quote-style = "double" + +# Like Black, indent with spaces, rather than tabs. +indent-style = "space" + +# Like Black, respect magic trailing commas. +skip-magic-trailing-comma = false + +# Like Black, automatically detect the appropriate line ending. +line-ending = "auto" [tool.isort] profile = "black" diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index b48a16c6..80636db8 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -64,6 +64,7 @@ def check_consistency( for check in ( check_containers_consistency, check_config_consistency, + check_resource_consistency, check_event_consistency, check_secrets_consistency, check_storages_consistency, @@ -92,6 +93,27 @@ def check_consistency( ) +def check_resource_consistency( + *, + state: "State", + charm_spec: "_CharmSpec", + **_kwargs, # noqa: U101 +) -> Results: + """Check the internal consistency of the resources from metadata and in State.""" + errors = [] + warnings = [] + + resources_from_meta = set(charm_spec.meta.get("resources", {})) + resources_from_state = set(state.resources) + if not resources_from_meta.issuperset(resources_from_state): + errors.append( + f"any and all resources passed to State.resources need to have been defined in " + f"metadata.yaml. Metadata resources: {resources_from_meta}; " + f"State.resources: {resources_from_state}.", + ) + return Results(errors, warnings) + + def check_event_consistency( *, event: "Event", @@ -396,7 +418,7 @@ def _get_relations(r): known_endpoints = [a[0] for a in all_relations_meta] for relation in state.relations: - if not (ep := relation.endpoint) in known_endpoints: + if (ep := relation.endpoint) not in known_endpoints: errors.append(f"relation endpoint {ep} is not declared in metadata.") seen_ids = set() diff --git a/scenario/mocking.py b/scenario/mocking.py index 1d4ff3a0..872a8134 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -6,11 +6,12 @@ import shutil from io import StringIO from pathlib import Path -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Set, Tuple, Union -from ops import pebble +from ops import JujuVersion, pebble from ops.model import ( ModelError, + RelationNotFoundError, SecretInfo, SecretNotFoundError, SecretRotate, @@ -38,6 +39,13 @@ logger = scenario_logger.getChild("mocking") +class ActionMissingFromContextError(Exception): + """Raised when the user attempts to invoke action hook tools outside an action context.""" + + # This is not an ops error: in ops, you'd have to go exceptionally out of your way to trigger + # this flow. + + class _MockExecProcess: def __init__(self, command: Tuple[str], change_id: int, out: "ExecOutput"): self._command = command @@ -123,8 +131,8 @@ def _get_relation_by_id( return next( filter(lambda r: r.relation_id == rel_id, self._state.relations), ) - except StopIteration as e: - raise RuntimeError(f"Not found: relation with id={rel_id}.") from e + except StopIteration: + raise RelationNotFoundError() def _get_secret(self, id=None, label=None): # cleanup id: @@ -142,6 +150,8 @@ def _get_secret(self, id=None, label=None): except StopIteration: raise SecretNotFoundError() else: + # if all goes well, this should never be reached. ops.model.Secret will check upon + # instantiation that either an id or a label are set, and raise a TypeError if not. raise RuntimeError("need id or label.") @staticmethod @@ -149,16 +159,30 @@ def _generate_secret_id(): id = "".join(map(str, [random.choice(list(range(10))) for _ in range(20)])) return f"secret:{id}" - def relation_get(self, rel_id, obj_name, app): - relation = self._get_relation_by_id(rel_id) - if app and obj_name == self.app_name: + def _check_app_data_access(self, is_app: bool): + if not isinstance(is_app, bool): + raise TypeError("is_app parameter to relation_get must be a boolean") + + if not is_app: + return + + version = JujuVersion(self._context.juju_version) + if not version.has_app_data(): + raise RuntimeError( + f"setting application data is not supported on Juju version {version}", + ) + + def relation_get(self, relation_id: int, member_name: str, is_app: bool): + self._check_app_data_access(is_app) + relation = self._get_relation_by_id(relation_id) + if is_app and member_name == self.app_name: return relation.local_app_data - elif app: + elif is_app: return relation.remote_app_data - elif obj_name == self.unit_name: + elif member_name == self.unit_name: return relation.local_unit_data - unit_id = int(obj_name.split("/")[-1]) + unit_id = int(member_name.split("/")[-1]) return relation._get_databag_for_remote(unit_id) # noqa def is_leader(self): @@ -206,6 +230,10 @@ def network_get(self, binding_name: str, relation_id: Optional[int] = None): if relation_id: logger.warning("network-get -r not implemented") + relations = self._state.get_relations(binding_name) + if not relations: + raise RelationNotFoundError() + network = next(filter(lambda r: r.name == binding_name, self._state.networks)) return network.hook_tool_output_fmt() @@ -225,9 +253,12 @@ def juju_log(self, level: str, message: str): self._context.juju_log.append(JujuLogLine(level, message)) def relation_set(self, relation_id: int, key: str, value: str, is_app: bool): + self._check_app_data_access(is_app) relation = self._get_relation_by_id(relation_id) if is_app: if not self._state.leader: + # will in practice not be reached because RelationData will check leadership + # and raise RelationDataAccessError upstream on this path raise RuntimeError("needs leadership to set app data") tgt = relation.local_app_data else: @@ -348,30 +379,35 @@ def secret_remove(self, id: str, *, revision: Optional[int] = None): else: secret.contents.clear() - def relation_remote_app_name(self, relation_id: int): - relation = self._get_relation_by_id(relation_id) + def relation_remote_app_name(self, relation_id: int) -> Optional[str]: + # ops catches relationnotfounderrors and returns None: + try: + relation = self._get_relation_by_id(relation_id) + except RelationNotFoundError: + return None return relation.remote_app_name def action_set(self, results: Dict[str, Any]): if not self._event.action: - raise RuntimeError( + raise ActionMissingFromContextError( "not in the context of an action event: cannot action-set", ) # let ops validate the results dict _format_action_result_dict(results) - # but then we will store it in its unformatted, original form + # but then we will store it in its unformatted, + # original form for testing ease self._context._action_results = results def action_fail(self, message: str = ""): if not self._event.action: - raise RuntimeError( + raise ActionMissingFromContextError( "not in the context of an action event: cannot action-fail", ) self._context._action_failure = message def action_log(self, message: str): if not self._event.action: - raise RuntimeError( + raise ActionMissingFromContextError( "not in the context of an action event: cannot action-log", ) self._context._action_logs.append(message) @@ -379,13 +415,19 @@ def action_log(self, message: str): def action_get(self): action = self._event.action if not action: - raise RuntimeError( + raise ActionMissingFromContextError( "not in the context of an action event: cannot action-get", ) return action.params def storage_add(self, name: str, count: int = 1): + if not isinstance(count, int) or isinstance(count, bool): + raise TypeError( + f"storage count must be integer, got: {count} ({type(count)})", + ) + if "/" in name: + # this error is raised by ops.testing but not by ops at runtime raise ModelError('storage name cannot contain "/"') self._context.requested_storages[name] = count @@ -395,8 +437,23 @@ def storage_list(self, name: str) -> List[int]: storage.index for storage in self._state.storage if storage.name == name ] + def _storage_event_details(self) -> Tuple[int, str]: + storage = self._event.storage + if not storage: + # only occurs if this method is called when outside the scope of a storage event + raise RuntimeError('unable to find storage key in ""') + fs_path = storage.get_filesystem(self._context) + return storage.index, str(fs_path) + def storage_get(self, storage_name_id: str, attribute: str) -> str: + if not len(attribute) > 0: # assume it's an empty string. + raise RuntimeError( + 'calling storage_get with `attribute=""` will return a dict ' + "and not a string. This usage is not supported.", + ) + if attribute != "location": + # this should not happen: in ops it's hardcoded to be "location" raise NotImplementedError( f"storage-get not implemented for attribute={attribute}", ) @@ -406,10 +463,11 @@ def storage_get(self, storage_name_id: str, attribute: str) -> str: storages: List[Storage] = [ s for s in self._state.storage if s.name == name and s.index == index ] + + # should not really happen: sanity checks. In practice, ops will guard against these paths. if not storages: raise RuntimeError(f"Storage with name={name} and index={index} not found.") if len(storages) > 1: - # should not really happen: sanity check. raise RuntimeError( f"Multiple Storage instances with name={name} and index={index} found. " f"Inconsistent state.", @@ -422,9 +480,37 @@ def storage_get(self, storage_name_id: str, attribute: str) -> str: def planned_units(self) -> int: return self._state.planned_units - # TODO: - def resource_get(self, *args, **kwargs): # noqa: U100 - raise NotImplementedError("resource_get") + # legacy ops API that we don't intend to mock: + def pod_spec_set( + self, + spec: Mapping[str, Any], # noqa: U100 + k8s_resources: Optional[Mapping[str, Any]] = None, # noqa: U100 + ): + raise NotImplementedError( + "pod-spec-set is not implemented in Scenario (and probably never will be: " + "it's deprecated API)", + ) + + def add_metrics( + self, + metrics: Mapping[str, Union[int, float]], # noqa: U100 + labels: Optional[Mapping[str, str]] = None, # noqa: U100 + ) -> None: + raise NotImplementedError( + "add-metrics is not implemented in Scenario (and probably never will be: " + "it's deprecated API)", + ) + + def resource_get(self, resource_name: str) -> str: + try: + return str(self._state.resources[resource_name]) + except KeyError: + # ops will not let us get there if the resource name is unknown from metadata. + # but if the user forgot to add it in State, then we remind you of that. + raise RuntimeError( + f"Inconsistent state: " + f"resource {resource_name} not found in State. please pass it.", + ) class _MockPebbleClient(_TestingPebbleClient): diff --git a/scenario/runtime.py b/scenario/runtime.py index dd64c416..41619c66 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -22,7 +22,7 @@ import yaml from ops.framework import EventBase, _event_regex -from ops.storage import SQLiteStorage +from ops.storage import NoSnapshotError, SQLiteStorage from scenario.capture_events import capture_events from scenario.logger import logger as scenario_logger @@ -100,10 +100,16 @@ def get_deferred_events(self) -> List["DeferredEvent"]: if EVENT_REGEX.match(handle_path): notices = db.notices(handle_path) for handle, owner, observer in notices: + try: + snapshot_data = db.load_snapshot(handle) + except NoSnapshotError: + snapshot_data = {} + event = DeferredEvent( handle_path=handle, owner=owner, observer=observer, + snapshot_data=snapshot_data, ) deferred.append(event) diff --git a/scenario/state.py b/scenario/state.py index 782f8bd8..df88724e 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -8,7 +8,7 @@ import re import typing from collections import namedtuple -from itertools import chain +from enum import Enum from pathlib import Path, PurePosixPath from typing import Any, Callable, Dict, List, Literal, Optional, Set, Tuple, Type, Union from uuid import uuid4 @@ -18,7 +18,6 @@ from ops.charm import CharmEvents from ops.model import SecretRotate, StatusBase -# from scenario.fs_mocks import _MockFileSystem, _MockStorageMount from scenario.logger import logger as scenario_logger JujuLogLine = namedtuple("JujuLogLine", ("level", "message")) @@ -46,6 +45,26 @@ DETACH_ALL_STORAGES = "DETACH_ALL_STORAGES" ACTION_EVENT_SUFFIX = "_action" +# all builtin events except secret events. They're special because they carry secret metadata. +BUILTIN_EVENTS = { + "start", + "stop", + "install", + "install", + "start", + "stop", + "remove", + "update_status", + "config_changed", + "upgrade_charm", + "pre_series_upgrade", + "post_series_upgrade", + "leader_elected", + "leader_settings_changed", + "collect_metrics", + "collect_app_status", + "collect_unit_status", +} PEBBLE_READY_EVENT_SUFFIX = "_pebble_ready" RELATION_EVENTS_SUFFIX = { "_relation_changed", @@ -756,10 +775,10 @@ def attached_event(self) -> "Event": ) @property - def detached_event(self) -> "Event": + def detaching_event(self) -> "Event": """Sugar to generate a -storage-detached event.""" return Event( - path=normalize_name(self.name + "-storage-detached"), + path=normalize_name(self.name + "-storage-detaching"), storage=self, ) @@ -796,6 +815,8 @@ class State(_DCBase): """The model this charm lives in.""" secrets: List[Secret] = dataclasses.field(default_factory=list) """The secrets this charm has access to (as an owner, or as a grantee).""" + resources: Dict[str, "PathLike"] = dataclasses.field(default_factory=dict) + """Mapping from resource name to path at which the resource can be found.""" planned_units: int = 1 """Number of non-dying planned units that are expected to be running this application. @@ -876,12 +897,24 @@ def get_container(self, container: Union[str, Container]) -> Container: except StopIteration as e: raise ValueError(f"container: {name}") from e - def get_relations(self, endpoint: str) -> Tuple["AnyRelation"]: - """Get relation from this State, based on an input relation or its endpoint name.""" - try: - return tuple(filter(lambda c: c.endpoint == endpoint, self.relations)) - except StopIteration as e: - raise ValueError(f"relation: {endpoint}") from e + def get_relations(self, endpoint: str) -> Tuple["AnyRelation", ...]: + """Get all relations on this endpoint from the current state.""" + + # we rather normalize the endpoint than worry about cursed metadata situations such as: + # requires: + # foo-bar: ... + # foo_bar: ... + + normalized_endpoint = normalize_name(endpoint) + return tuple( + r + for r in self.relations + if normalize_name(r.endpoint) == normalized_endpoint + ) + + def get_storages(self, name: str) -> Tuple["Storage", ...]: + """Get all storages with this name.""" + return tuple(s for s in self.storage if s.name == name) # FIXME: not a great way to obtain a delta, but is "complete". todo figure out a better way. def jsonpatch_delta(self, other: "State"): @@ -964,6 +997,62 @@ def name(self): return self.handle_path.split("/")[-1].split("[")[0] +class _EventType(str, Enum): + builtin = "builtin" + relation = "relation" + action = "action" + secret = "secret" + storage = "storage" + workload = "workload" + custom = "custom" + + +class _EventPath(str): + def __new__(cls, string): + string = normalize_name(string) + instance = super().__new__(cls, string) + + instance.name = name = string.split(".")[-1] + instance.owner_path = string.split(".")[:-1] or ["on"] + + instance.suffix, instance.type = suffix, _ = _EventPath._get_suffix_and_type( + name, + ) + if suffix: + instance.prefix, _ = string.rsplit(suffix) + else: + instance.prefix = string + + instance.is_custom = suffix == "" + return instance + + @staticmethod + def _get_suffix_and_type(s: str): + for suffix in RELATION_EVENTS_SUFFIX: + if s.endswith(suffix): + return suffix, _EventType.relation + + if s.endswith(ACTION_EVENT_SUFFIX): + return ACTION_EVENT_SUFFIX, _EventType.action + + if s in SECRET_EVENTS: + return s, _EventType.secret + + # Whether the event name indicates that this is a storage event. + for suffix in STORAGE_EVENTS_SUFFIX: + if s.endswith(suffix): + return suffix, _EventType.storage + + # Whether the event name indicates that this is a workload event. + if s.endswith(PEBBLE_READY_EVENT_SUFFIX): + return PEBBLE_READY_EVENT_SUFFIX, _EventType.workload + + if s in BUILTIN_EVENTS: + return "", _EventType.builtin + + return "", _EventType.custom + + @dataclasses.dataclass(frozen=True) class Event(_DCBase): path: str @@ -1002,14 +1091,27 @@ def __call__(self, remote_unit_id: Optional[int] = None) -> "Event": return self.replace(relation_remote_unit_id=remote_unit_id) def __post_init__(self): - path = normalize_name(self.path) + path = _EventPath(self.path) # bypass frozen dataclass object.__setattr__(self, "path", path) + @property + def _path(self) -> _EventPath: + # we converted it in __post_init__, but the type checker doesn't know about that + return typing.cast(_EventPath, self.path) + @property def name(self) -> str: - """Event name.""" - return self.path.split(".")[-1] + """Full event name. + + Consists of a 'prefix' and a 'suffix'. The suffix denotes the type of the event, the + prefix the name of the entity the event is about. + + "foo-relation-changed": + - "foo"=prefix (name of a relation), + - "-relation-changed"=suffix (relation event) + """ + return self._path.name @property def owner_path(self) -> List[str]: @@ -1017,32 +1119,32 @@ def owner_path(self) -> List[str]: If this event is defined on the toplevel charm class, it should be ['on']. """ - return self.path.split(".")[:-1] or ["on"] + return self._path.owner_path @property def _is_relation_event(self) -> bool: """Whether the event name indicates that this is a relation event.""" - return any(self.name.endswith(suffix) for suffix in RELATION_EVENTS_SUFFIX) + return self._path.type is _EventType.relation @property def _is_action_event(self) -> bool: """Whether the event name indicates that this is a relation event.""" - return self.name.endswith(ACTION_EVENT_SUFFIX) + return self._path.type is _EventType.action @property def _is_secret_event(self) -> bool: """Whether the event name indicates that this is a secret event.""" - return self.name in SECRET_EVENTS + return self._path.type is _EventType.secret @property def _is_storage_event(self) -> bool: """Whether the event name indicates that this is a storage event.""" - return any(self.name.endswith(suffix) for suffix in STORAGE_EVENTS_SUFFIX) + return self._path.type is _EventType.storage @property def _is_workload_event(self) -> bool: """Whether the event name indicates that this is a workload event.""" - return self.name.endswith("_pebble_ready") + return self._path.type is _EventType.workload # this method is private because _CharmSpec is not quite user-facing; also, # the user should know. @@ -1062,31 +1164,8 @@ def _is_builtin_event(self, charm_spec: "_CharmSpec"): # `charm.lib.on.foo_relation_created` and therefore be unique and the Framework is happy. # However, our Event data structure ATM has no knowledge of which Object/Handle it is # owned by. So the only thing we can do right now is: check whether the event name, - # assuming it is owned by the charm, is that of a builtin event or not. - builtins = [] - for relation_name in chain( - charm_spec.meta.get("requires", ()), - charm_spec.meta.get("provides", ()), - charm_spec.meta.get("peers", ()), - ): - relation_name = relation_name.replace("-", "_") - for relation_evt_suffix in RELATION_EVENTS_SUFFIX: - builtins.append(relation_name + relation_evt_suffix) - - for storage_name in charm_spec.meta.get("storages", ()): - storage_name = storage_name.replace("-", "_") - for storage_evt_suffix in STORAGE_EVENTS_SUFFIX: - builtins.append(storage_name + storage_evt_suffix) - - for action_name in charm_spec.actions or (): - action_name = action_name.replace("-", "_") - builtins.append(action_name + ACTION_EVENT_SUFFIX) - - for container_name in charm_spec.meta.get("containers", ()): - container_name = container_name.replace("-", "_") - builtins.append(container_name + PEBBLE_READY_EVENT_SUFFIX) - - return event_name in builtins + # 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. @@ -1094,8 +1173,11 @@ def bind(self, state: State): 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.name.split("_")[0] + entity_name = self._path.prefix if self._is_workload_event and not self.container: try: @@ -1113,6 +1195,19 @@ def bind(self, state: State): ) return self.replace(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 self.replace(storage=storage) + if self._is_relation_event and not self.relation: ep_name = entity_name relations = state.get_relations(ep_name) diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 8afc84c9..bcdc7df8 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -426,3 +426,44 @@ def test_storage_states(): }, ), ) + + +def test_resource_states(): + # happy path + assert_consistent( + State(resources={"foo": "/foo/bar.yaml"}), + Event("start"), + _CharmSpec( + MyCharm, + meta={"name": "yamlman", "resources": {"foo": {"type": "oci-image"}}}, + ), + ) + + # no resources in state but some in meta: OK. Not realistic wrt juju but fine for testing + assert_consistent( + State(), + Event("start"), + _CharmSpec( + MyCharm, + meta={"name": "yamlman", "resources": {"foo": {"type": "oci-image"}}}, + ), + ) + + # resource not defined in meta + assert_inconsistent( + State(resources={"bar": "/foo/bar.yaml"}), + Event("start"), + _CharmSpec( + MyCharm, + meta={"name": "yamlman", "resources": {"foo": {"type": "oci-image"}}}, + ), + ) + + assert_inconsistent( + State(resources={"bar": "/foo/bar.yaml"}), + Event("start"), + _CharmSpec( + MyCharm, + meta={"name": "yamlman"}, + ), + ) diff --git a/tests/test_e2e/test_deferred.py b/tests/test_e2e/test_deferred.py index 6593d9b9..bdd70db0 100644 --- a/tests/test_e2e/test_deferred.py +++ b/tests/test_e2e/test_deferred.py @@ -10,6 +10,7 @@ ) from ops.framework import Framework +from scenario import Context from scenario.state import Container, DeferredEvent, Relation, State, deferred from tests.helpers import trigger @@ -34,7 +35,7 @@ def __init__(self, framework: Framework): def _on_event(self, event): self.captured.append(event) - if self.defer_next: + if self.defer_next > 0: self.defer_next -= 1 return event.defer() @@ -134,7 +135,9 @@ def test_deferred_relation_event_from_relation(mycharm): out = trigger( State( relations=[rel], - deferred=[rel.changed_event.deferred(handler=mycharm._on_event)], + deferred=[ + rel.changed_event(remote_unit_id=1).deferred(handler=mycharm._on_event) + ], ), "start", mycharm, @@ -144,6 +147,12 @@ def test_deferred_relation_event_from_relation(mycharm): # we deferred the first 2 events we saw: foo_relation_changed, start. assert len(out.deferred) == 2 assert out.deferred[0].name == "foo_relation_changed" + assert out.deferred[0].snapshot_data == { + "relation_name": rel.endpoint, + "relation_id": rel.relation_id, + "app_name": "remote", + "unit_name": "remote/1", + } assert out.deferred[1].name == "start" # we saw start and foo_relation_changed. @@ -178,3 +187,40 @@ def test_deferred_workload_event(mycharm): upstat, start = mycharm.captured assert isinstance(upstat, WorkloadEvent) assert isinstance(start, StartEvent) + + +def test_defer_reemit_lifecycle_event(mycharm): + ctx = Context(mycharm, meta=mycharm.META) + + mycharm.defer_next = 1 + state_1 = ctx.run("update-status", State()) + + mycharm.defer_next = 0 + state_2 = ctx.run("start", state_1) + + assert [type(e).__name__ for e in ctx.emitted_events] == [ + "UpdateStatusEvent", + "UpdateStatusEvent", + "StartEvent", + ] + assert len(state_1.deferred) == 1 + assert not state_2.deferred + + +def test_defer_reemit_relation_event(mycharm): + ctx = Context(mycharm, meta=mycharm.META) + + rel = Relation("foo") + mycharm.defer_next = 1 + state_1 = ctx.run(rel.created_event, State(relations=[rel])) + + mycharm.defer_next = 0 + state_2 = ctx.run("start", state_1) + + assert [type(e).__name__ for e in ctx.emitted_events] == [ + "RelationCreatedEvent", + "RelationCreatedEvent", + "StartEvent", + ] + assert len(state_1.deferred) == 1 + assert not state_2.deferred diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py new file mode 100644 index 00000000..2ce9b5aa --- /dev/null +++ b/tests/test_e2e/test_event.py @@ -0,0 +1,50 @@ +import pytest +from ops import CharmBase + +from scenario.state import Event, _CharmSpec, _EventType + + +@pytest.mark.parametrize( + "evt, expected_type", + ( + ("foo_relation_changed", _EventType.relation), + ("foo_relation_created", _EventType.relation), + ("foo_bar_baz_relation_created", _EventType.relation), + ("foo_storage_attached", _EventType.storage), + ("foo_storage_detaching", _EventType.storage), + ("foo_bar_baz_storage_detaching", _EventType.storage), + ("foo_pebble_ready", _EventType.workload), + ("foo_bar_baz_pebble_ready", _EventType.workload), + ("secret_removed", _EventType.secret), + ("foo", _EventType.custom), + ("kaboozle_bar_baz", _EventType.custom), + ), +) +def test_event_type(evt, expected_type): + event = Event(evt) + assert event._path.type is expected_type + + assert event._is_relation_event is (expected_type is _EventType.relation) + assert event._is_storage_event is (expected_type is _EventType.storage) + assert event._is_workload_event is (expected_type is _EventType.workload) + assert event._is_secret_event is (expected_type is _EventType.secret) + assert event._is_action_event is (expected_type is _EventType.action) + + class MyCharm(CharmBase): + pass + + spec = _CharmSpec( + MyCharm, + meta={ + "requires": { + "foo": {"interface": "bar"}, + "foo_bar_baz": {"interface": "bar"}, + }, + "storage": { + "foo": {"type": "filesystem"}, + "foo_bar_baz": {"type": "filesystem"}, + }, + "containers": {"foo": {}, "foo_bar_baz": {}}, + }, + ) + assert event._is_builtin_event(spec) is (expected_type is not _EventType.custom) diff --git a/tests/test_e2e/test_event_bind.py b/tests/test_e2e/test_event_bind.py index 816b1c8c..4878e6ac 100644 --- a/tests/test_e2e/test_event_bind.py +++ b/tests/test_e2e/test_event_bind.py @@ -11,6 +11,13 @@ def test_bind_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=[]) diff --git a/tests/test_e2e/test_network.py b/tests/test_e2e/test_network.py index fccd6029..8c10c6cc 100644 --- a/tests/test_e2e/test_network.py +++ b/tests/test_e2e/test_network.py @@ -1,4 +1,5 @@ import pytest +from ops import RelationNotFoundError from ops.charm import CharmBase from ops.framework import Framework @@ -53,3 +54,33 @@ def fetch_unit_address(charm: CharmBase): }, post_event=fetch_unit_address, ) + + +def test_no_relation_error(mycharm): + """Attempting to call get_binding on a non-existing relation -> RelationNotFoundError""" + mycharm._call = lambda *_: True + + def fetch_unit_address(charm: CharmBase): + with pytest.raises(RelationNotFoundError): + _ = charm.model.get_binding("foo").network + + trigger( + State( + relations=[ + Relation( + interface="foo", + remote_app_name="remote", + endpoint="metrics-endpoint", + relation_id=1, + ), + ], + networks=[Network.default("metrics-endpoint")], + ), + "update_status", + mycharm, + meta={ + "name": "foo", + "requires": {"metrics-endpoint": {"interface": "foo"}}, + }, + post_event=fetch_unit_address, + ) diff --git a/tests/test_e2e/test_rubbish_events.py b/tests/test_e2e/test_rubbish_events.py index ad95fbf7..10582d82 100644 --- a/tests/test_e2e/test_rubbish_events.py +++ b/tests/test_e2e/test_rubbish_events.py @@ -79,7 +79,7 @@ def test_custom_events_sub_raise(mycharm, evt_name): ("install", True), ("config-changed", True), ("foo-relation-changed", True), - ("bar-relation-changed", False), + ("bar-relation-changed", True), ), ) def test_is_custom_event(mycharm, evt_name, expected): diff --git a/tests/test_e2e/test_storage.py b/tests/test_e2e/test_storage.py index a33893f7..87aa9370 100644 --- a/tests/test_e2e/test_storage.py +++ b/tests/test_e2e/test_storage.py @@ -79,3 +79,13 @@ def test_storage_usage(storage_ctx): assert ( storage.get_filesystem(storage_ctx) / "path.py" ).read_text() == "helloworlds" + + +def test_storage_attached_event(storage_ctx): + storage = Storage("foo") + storage_ctx.run(storage.attached_event, State(storage=[storage])) + + +def test_storage_detaching_event(storage_ctx): + storage = Storage("foo") + storage_ctx.run(storage.detaching_event, State(storage=[storage])) diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 0f37ce82..60e6293e 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -103,7 +103,8 @@ def test_env_cleanup_on_charm_error(): state=State(), event=Event("box_relation_changed", relation=Relation("box")), context=Context(my_charm_type, meta=meta), - ) as charm: + ): assert os.getenv("JUJU_REMOTE_APP") + _ = 1 / 0 # raise some error assert os.getenv("JUJU_REMOTE_APP", None) is None diff --git a/tox.ini b/tox.ini index 9e052fb5..da9c40ac 100644 --- a/tox.ini +++ b/tox.ini @@ -54,8 +54,8 @@ commands = description = Format code. skip_install = true deps = - black + ruff isort commands = - black {[vars]tst_path} {[vars]src_path} + ruff format {[vars]tst_path} {[vars]src_path} isort --profile black {[vars]tst_path} {[vars]src_path}