fixed error types and added resource mocks

README.md

@@ -1045,6 +1045,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`,

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 = "[email protected]" }

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", {}).keys())
+    resources_from_state = set(state.resources.keys())
+    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",

scenario/mocking.py

@@ -6,9 +6,9 @@
 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,
@@ -131,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 RelationNotFoundError(f"Not found: relation with id={rel_id}.") from e
+        except StopIteration:
+            raise RelationNotFoundError()
 
     def _get_secret(self, id=None, label=None):
         # cleanup id:
@@ -150,23 +150,37 @@ 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
     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 is_app:
+            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):
@@ -214,6 +228,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()
 
@@ -233,9 +251,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:
@@ -356,8 +377,12 @@ 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]):
@@ -367,7 +392,8 @@ def action_set(self, results: Dict[str, Any]):
             )
         # 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 = ""):
@@ -393,7 +419,13 @@ def action_get(self):
         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
@@ -403,8 +435,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}",
             )
@@ -414,10 +461,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.",
@@ -430,9 +478,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):

scenario/state.py

@@ -756,10 +756,10 @@ def attached_event(self) -> "Event":
         )
 
     @property
-    def detached_event(self) -> "Event":
+    def detaching_event(self) -> "Event":
         """Sugar to generate a <this storage>-storage-detached event."""
         return Event(
-            path=normalize_name(self.name + "-storage-detached"),
+            path=normalize_name(self.name + "-storage-detaching"),
             storage=self,
         )
 
@@ -796,6 +796,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 +878,13 @@ 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."""
+        return tuple(filter(lambda c: c.endpoint == endpoint, self.relations))
+
+    def get_storages(self, name: str) -> Tuple["Storage", ...]:
+        """Get all storages with this name."""
+        return tuple(filter(lambda s: s.name == name, self.storage))
 
     # FIXME: not a great way to obtain a delta, but is "complete". todo figure out a better way.
     def jsonpatch_delta(self, other: "State"):
@@ -1094,6 +1097,9 @@ 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]
 
@@ -1113,6 +1119,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)

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"},
+        ),
+    )