Merge pull request #194 from tonyandrewmeyer/fix-get_relation-keyerro…

…r-193

fix: ops.Model.get_relation should not raise when a relation with the specified ID does not exist

pyproject.toml

@@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta"
 [project]
 name = "ops-scenario"
 
-version = "7.0.2"
+version = "7.0.3"
 
 authors = [
     { name = "Pietro Pasotti", email = "[email protected]" }

scenario/mocking.py

@@ -181,7 +181,7 @@ def get_pebble(self, socket_path: str) -> "Client":
     def _get_relation_by_id(self, rel_id) -> "RelationBase":
         try:
             return self._state.get_relation(rel_id)
-        except ValueError:
+        except KeyError:
             raise RelationNotFoundError() from None
 
     def _get_secret(self, id=None, label=None):

tests/test_e2e/test_relations.py

@@ -13,6 +13,7 @@
 from ops.framework import EventBase, Framework
 
 from scenario import Context
+from scenario.errors import UncaughtCharmError
 from scenario.state import (
     _DEFAULT_JUJU_DATABAG,
     PeerRelation,
@@ -424,6 +425,52 @@ def test_broken_relation_not_in_model_relations(mycharm):
         assert charm.model.relations["foo"] == []
 
 
+def test_get_relation_when_missing():
+    class MyCharm(CharmBase):
+        def __init__(self, framework):
+            super().__init__(framework)
+            self.framework.observe(self.on.update_status, self._on_update_status)
+            self.framework.observe(self.on.config_changed, self._on_config_changed)
+            self.relation = None
+
+        def _on_update_status(self, _):
+            self.relation = self.model.get_relation("foo")
+
+        def _on_config_changed(self, _):
+            self.relation = self.model.get_relation("foo", self.config["relation-id"])
+
+    ctx = Context(
+        MyCharm,
+        meta={"name": "foo", "requires": {"foo": {"interface": "foo"}}},
+        config={"options": {"relation-id": {"type": "int", "description": "foo"}}},
+    )
+    # There should be no error if the relation is missing - get_relation returns
+    # None in that case.
+    with ctx(ctx.on.update_status(), State()) as mgr:
+        mgr.run()
+        assert mgr.charm.relation is None
+
+    # There should also be no error if the relation is present, of course.
+    rel = Relation("foo")
+    with ctx(ctx.on.update_status(), State(relations={rel})) as mgr:
+        mgr.run()
+        assert mgr.charm.relation.id == rel.id
+
+    # If a relation that doesn't exist is requested, that should also not raise
+    # an error.
+    with ctx(ctx.on.config_changed(), State(config={"relation-id": 42})) as mgr:
+        mgr.run()
+        rel = mgr.charm.relation
+        assert rel.id == 42
+        assert not rel.active
+
+    # If there's no defined relation with the name, then get_relation raises KeyError.
+    ctx = Context(MyCharm, meta={"name": "foo"})
+    with pytest.raises(UncaughtCharmError) as exc:
+        ctx.run(ctx.on.update_status(), State())
+    assert isinstance(exc.value.__cause__, KeyError)
+
+
 @pytest.mark.parametrize("klass", (Relation, PeerRelation, SubordinateRelation))
 def test_relation_positional_arguments(klass):
     with pytest.raises(TypeError):