From 470a535b2c5ce8a71a97096634cac982a27b0d42 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Fri, 25 Aug 2023 09:27:37 +1200 Subject: [PATCH] Simplify Harness.add_relation by adding app_data and unit_data args (#994) When using testing.Harness, it's very common to see these two or three calls in a row: add relation, (often) add relation unit, and (often) update relation data. This addition simplifies those patterns down to just a single call, and without the need for the relation_id return value in most cases. Fixes #992 --- README.md | 24 +++++---- ops/testing.py | 124 +++++++++++++++++++++++++++++++++---------- test/test_testing.py | 32 +++++++++++ 3 files changed, 142 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 8fa10ef1d..b8cd27a96 100644 --- a/README.md +++ b/README.md @@ -56,15 +56,21 @@ a full deployment. Our [API documentation](https://ops.readthedocs.io/en/latest/ has the details, including this example: ```python -harness = Harness(MyCharm) -# Do initial setup here -relation_id = harness.add_relation('db', 'postgresql') -# Now instantiate the charm to see events as the model changes -harness.begin() -harness.add_relation_unit(relation_id, 'postgresql/0') -harness.update_relation_data(relation_id, 'postgresql/0', {'key': 'val'}) -# Check that charm has properly handled the relation_joined event for postgresql/0 -self.assertEqual(harness.charm. ...) +class TestCharm(unittest.TestCase): + def test_foo(self): + harness = Harness(MyCharm) + self.addCleanup(harness.cleanup) # always clean up after ourselves + + # Instantiate the charm and trigger events that Juju would on startup + harness.begin_with_initial_hooks() + + # Update charm config and trigger config-changed + harness.update_config({'log_level': 'warn'}) + + # Check that charm properly handled config-changed, for example, + # the charm added the correct Pebble layer + plan = harness.get_container_pebble_plan('prometheus') + self.assertIn('--log.level=warn', plan.services['prometheus'].command) ``` diff --git a/ops/testing.py b/ops/testing.py index 4247d053e..970d86099 100755 --- a/ops/testing.py +++ b/ops/testing.py @@ -149,23 +149,49 @@ class ExecResult: class Harness(Generic[CharmType]): """This class represents a way to build up the model that will drive a test suite. - The model that is created is from the viewpoint of the charm that you are testing. + The model created is from the viewpoint of the charm that you are testing. - Example:: + Below is an example test using :meth:`begin_with_initial_hooks` that ensures + the charm responds correctly to config changes:: - harness = Harness(MyCharm) - self.addCleanup(harness.cleanup) # always clean up after ourselves + class TestCharm(unittest.TestCase): + def test_foo(self): + harness = Harness(MyCharm) + self.addCleanup(harness.cleanup) # always clean up after ourselves - # Do initial setup here - relation_id = harness.add_relation('db', 'postgresql') + # Instantiate the charm and trigger events that Juju would on startup + harness.begin_with_initial_hooks() - # Now instantiate the charm to see events as the model changes - harness.begin() - harness.add_relation_unit(relation_id, 'postgresql/0') - harness.update_relation_data(relation_id, 'postgresql/0', {'key': 'val'}) + # Update charm config and trigger config-changed + harness.update_config({'log_level': 'warn'}) - # Check that charm has properly handled the relation_joined event for postgresql/0 - self.assertEqual(harness.charm. ...) + # Check that charm properly handled config-changed, for example, + # the charm added the correct Pebble layer + plan = harness.get_container_pebble_plan('prometheus') + self.assertIn('--log.level=warn', plan.services['prometheus'].command) + + To set up the model without triggering events (or calling charm code), perform the + harness actions before calling :meth:`begin`. Below is an example that adds a + relation before calling ``begin``, and then updates config to trigger the + ``config-changed`` event in the charm:: + + class TestCharm(unittest.TestCase): + def test_bar(self): + harness = Harness(MyCharm) + self.addCleanup(harness.cleanup) # always clean up after ourselves + + # Set up model before "begin" (no events triggered) + harness.set_leader(True) + harness.add_relation('db', 'postgresql', unit_data={'key': 'val'}) + + # Now instantiate the charm to start triggering events as the model changes + harness.begin() + harness.update_config({'some': 'config'}) + + # Check that charm has properly handled config-changed, for example, + # has written the app's config file + root = harness.get_filesystem_root('container') + assert (root / 'etc' / 'app.conf').exists() Args: charm_cls: The Charm class that you'll be testing. @@ -336,9 +362,7 @@ def begin_with_initial_hooks(self) -> None: # Add storage if needed before begin_with_initial_hooks() is called storage_ids = harness.add_storage('data', count=1)[0] storage_id = storage_id[0] # we only added one storage instance - relation_id = harness.add_relation('db', 'postgresql') - harness.add_relation_unit(relation_id, 'postgresql/0') - harness.update_relation_data(relation_id, 'postgresql/0', {'key': 'val'}) + harness.add_relation('db', 'postgresql', unit_data={'key': 'val'}) harness.set_leader(True) harness.update_config({'initial': 'config'}) harness.begin_with_initial_hooks() @@ -737,20 +761,45 @@ def remove_storage(self, storage_id: str) -> None: model.Storage(storage_name, storage_index, self._backend)) self._backend._storage_remove(storage_id) - def add_relation(self, relation_name: str, remote_app: str) -> int: - """Declare that there is a new relation between this app and `remote_app`. + def add_relation(self, relation_name: str, remote_app: str, *, + app_data: Optional[Mapping[str, str]] = None, + unit_data: Optional[Mapping[str, str]] = None) -> int: + """Declare that there is a new relation between this application and `remote_app`. - In the case of adding peer relations, `remote_app` is *this* app. This function creates a - relation with an application and will trigger a relation-created hook. To relate units (and - trigger relation-joined and relation-changed hooks), you should also call - :meth:`.add_relation_unit`. + This function creates a relation with an application and triggers a + :class:`RelationCreatedEvent `. + + If `app_data` or `unit_data` are provided, also add a new unit + (``/0``) to the relation and trigger + :class:`RelationJoinedEvent `. Then update + the application data if `app_data` is provided and the unit data if + `unit_data` is provided, triggering + :class:`RelationChangedEvent ` after each update. + Alternatively, charm tests can call :meth:`add_relation_unit` and + :meth:`update_relation_data` explicitly. + + Example usage:: + + secret_id = harness.add_model_secret('mysql', {'password': 'SECRET'}) + harness.add_relation('db', 'mysql', unit_data={ + 'host': 'mysql.localhost, + 'username': 'appuser', + 'secret-id': secret_id, + }) Args: - relation_name: The relation on Charm that is being related to - remote_app: The name of the application that is being related to + relation_name: The relation on the charm that is being related to. + remote_app: The name of the application that is being related to. + To add a peer relation, set to the name of *this* application. + app_data: If provided, also add a new unit to the relation + (triggering relation-joined) and set the *application* relation data + (triggering relation-changed). + unit_data: If provided, also add a new unit to the relation + (triggering relation-joined) and set the *unit* relation data + (triggering relation-changed). Return: - The relation_id created by this add_relation. + The ID of the relation created. """ relation_id = self._next_relation_id() self._backend._relation_ids_map.setdefault( @@ -770,6 +819,15 @@ def add_relation(self, relation_name: str, remote_app: str) -> int: if self._model is not None: self._model.relations._invalidate(relation_name) self._emit_relation_created(relation_name, relation_id, remote_app) + + if app_data is not None or unit_data is not None: + remote_unit = remote_app + '/0' + self.add_relation_unit(relation_id, remote_unit) + if app_data is not None: + self.update_relation_data(relation_id, remote_app, app_data) + if unit_data is not None: + self.update_relation_data(relation_id, remote_unit, unit_data) + return relation_id def remove_relation(self, relation_id: int) -> None: @@ -830,17 +888,21 @@ def _emit_relation_broken(self, relation_name: str, relation_id: int, def add_relation_unit(self, relation_id: int, remote_unit_name: str) -> None: """Add a new unit to a relation. - Example:: - - rel_id = harness.add_relation('db', 'postgresql') - harness.add_relation_unit(rel_id, 'postgresql/0') - This will trigger a `relation_joined` event. This would naturally be followed by a `relation_changed` event, which you can trigger with :meth:`.update_relation_data`. This separation is artificial in the sense that Juju will always fire the two, but is intended to make testing relations and their data bags slightly more natural. + Unless finer-grained control is needed, most charm tests can call + :meth:`add_relation` with the `app_data` or `unit_data` argument + instead of using this function. + + Example:: + + rel_id = harness.add_relation('db', 'postgresql') + harness.add_relation_unit(rel_id, 'postgresql/0') + Args: relation_id: The integer relation identifier (as returned by :meth:`add_relation`). remote_unit_name: A string representing the remote unit that is being added. @@ -1058,6 +1120,10 @@ def update_relation_data( This also triggers the `relation_changed` event for the given ``relation_id``. + Unless finer-grained control is needed, most charm tests can call + :meth:`add_relation` with the `app_data` or `unit_data` argument + instead of using this function. + Args: relation_id: The integer relation ID representing this relation. app_or_unit: The unit or application name that is being updated. diff --git a/test/test_testing.py b/test/test_testing.py index b37d8590d..df24292e0 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -108,6 +108,38 @@ def test_add_relation(self): self.assertEqual(backend.relation_get(rel_id, 'test-app', is_app=True), {}) self.assertEqual(backend.relation_get(rel_id, 'test-app/0', is_app=False), {}) + def test_add_relation_with_app_data(self): + harness = ops.testing.Harness(ops.CharmBase, meta=''' + name: test-app + requires: + db: + interface: pgsql + ''') + self.addCleanup(harness.cleanup) + rel_id = harness.add_relation('db', 'postgresql', app_data={'x': '1', 'y': '2'}) + self.assertIsInstance(rel_id, int) + backend = harness._backend + self.assertEqual(backend.relation_ids('db'), [rel_id]) + self.assertEqual(backend.relation_list(rel_id), ['postgresql/0']) + self.assertEqual(harness.get_relation_data(rel_id, 'postgresql'), {'x': '1', 'y': '2'}) + self.assertEqual(harness.get_relation_data(rel_id, 'postgresql/0'), {}) + + def test_add_relation_with_unit_data(self): + harness = ops.testing.Harness(ops.CharmBase, meta=''' + name: test-app + requires: + db: + interface: pgsql + ''') + self.addCleanup(harness.cleanup) + rel_id = harness.add_relation('db', 'postgresql', unit_data={'a': '1', 'b': '2'}) + self.assertIsInstance(rel_id, int) + backend = harness._backend + self.assertEqual(backend.relation_ids('db'), [rel_id]) + self.assertEqual(backend.relation_list(rel_id), ['postgresql/0']) + self.assertEqual(harness.get_relation_data(rel_id, 'postgresql'), {}) + self.assertEqual(harness.get_relation_data(rel_id, 'postgresql/0'), {'a': '1', 'b': '2'}) + def test_can_connect_default(self): harness = ops.testing.Harness(ops.CharmBase, meta=''' name: test-app