From 223ee464c586e191e93e0088cf4af6dc36990d00 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 2 Oct 2023 09:24:41 +1300 Subject: [PATCH 1/3] Ensure we don't backslide. --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index f828007d2..33393ad4c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ include = ["ops/*.py", "ops/_private/*.py", "test/test_lib.py", "test/test_model.py", "test/test_testing.py", + "test/test_charm.py", ] pythonVersion = "3.8" # check no python > 3.8 features are used pythonPlatform = "All" From 38966f98d64327bf1354123c1307f38ac52a1bfc Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 2 Oct 2023 10:33:20 +1300 Subject: [PATCH 2/3] Add type hints. --- test/test_charm.py | 118 +++++++++++++++++++++++---------------------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/test/test_charm.py b/test/test_charm.py index d28b042db..71d12c149 100755 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -15,6 +15,7 @@ import os import shutil import tempfile +import typing import unittest from pathlib import Path @@ -29,7 +30,7 @@ class TestCharm(unittest.TestCase): def setUp(self): - def restore_env(env): + def restore_env(env: typing.Dict[str, str]): os.environ.clear() os.environ.update(env) self.addCleanup(restore_env, os.environ.copy()) @@ -51,10 +52,10 @@ class TestCharmEvents(ops.CharmEvents): # Relations events are defined dynamically and modify the class attributes. # We use a subclass temporarily to prevent these side effects from leaking. - ops.CharmBase.on = TestCharmEvents() + ops.CharmBase.on = TestCharmEvents() # type: ignore def cleanup(): - ops.CharmBase.on = ops.CharmEvents() + ops.CharmBase.on = ops.CharmEvents() # type: ignore self.addCleanup(cleanup) def create_framework(self): @@ -69,16 +70,16 @@ def test_basic(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.started = False framework.observe(self.on.start, self._on_start) - def _on_start(self, event): + def _on_start(self, event: ops.EventBase): self.started = True - events = list(MyCharm.on.events()) + events: typing.List[str] = list(MyCharm.on.events()) # type: ignore self.assertIn('install', events) self.assertIn('custom', events) @@ -89,7 +90,7 @@ def _on_start(self, event): self.assertEqual(charm.started, True) with self.assertRaisesRegex(TypeError, "observer methods must now be explicitly provided"): - framework.observe(charm.on.start, charm) + framework.observe(charm.on.start, charm) # type: ignore def test_observe_decorated_method(self): # we test that charm methods decorated with @functools.wraps(wrapper) @@ -98,25 +99,25 @@ def test_observe_decorated_method(self): # is more careful and it still works, this test is here to ensure that # it keeps working in future releases, as this is presently the only # way we know of to cleanly decorate charm event observers. - events = [] + events: typing.List[ops.EventBase] = [] - def dec(fn): + def dec(fn: typing.Callable[['MyCharm', ops.EventBase], None]): # simple decorator that appends to the nonlocal # `events` list all events it receives @functools.wraps(fn) - def wrapper(charm, evt): + def wrapper(charm: 'MyCharm', evt: ops.EventBase): events.append(evt) fn(charm, evt) return wrapper class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) framework.observe(self.on.start, self._on_start) self.seen = None @dec - def _on_start(self, event): + def _on_start(self, event: ops.EventBase): self.seen = event framework = self.create_framework() @@ -147,9 +148,9 @@ class MyCharm(ops.CharmBase): def test_relation_events(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) - self.seen = [] + self.seen: typing.List[str] = [] for rel in ('req1', 'req-2', 'pro1', 'pro-2', 'peer1', 'peer-2'): # Hook up relation events to generic handler. self.framework.observe(self.on[rel].relation_joined, self.on_any_relation) @@ -157,8 +158,9 @@ def __init__(self, *args): self.framework.observe(self.on[rel].relation_departed, self.on_any_relation) self.framework.observe(self.on[rel].relation_broken, self.on_any_relation) - def on_any_relation(self, event): + def on_any_relation(self, event: ops.RelationEvent): assert event.relation.name == 'req1' + assert event.relation.app is not None assert event.relation.app.name == 'remote' self.seen.append(type(event).__name__) @@ -210,25 +212,25 @@ def test_storage_events(self): this = self class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) - self.seen = [] + self.seen: typing.List[str] = [] self.framework.observe(self.on['stor1'].storage_attached, self._on_stor1_attach) self.framework.observe(self.on['stor2'].storage_detaching, self._on_stor2_detach) self.framework.observe(self.on['stor3'].storage_attached, self._on_stor3_attach) self.framework.observe(self.on['stor-4'].storage_attached, self._on_stor4_attach) - def _on_stor1_attach(self, event): + def _on_stor1_attach(self, event: ops.StorageAttachedEvent): self.seen.append(type(event).__name__) this.assertEqual(event.storage.location, Path("/var/srv/stor1/0")) - def _on_stor2_detach(self, event): + def _on_stor2_detach(self, event: ops.StorageDetachingEvent): self.seen.append(type(event).__name__) - def _on_stor3_attach(self, event): + def _on_stor3_attach(self, event: ops.StorageAttachedEvent): self.seen.append(type(event).__name__) - def _on_stor4_attach(self, event): + def _on_stor4_attach(self, event: ops.StorageAttachedEvent): self.seen.append(type(event).__name__) # language=YAML @@ -320,9 +322,9 @@ def _on_stor4_attach(self, event): def test_workload_events(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) - self.seen = [] + self.seen: typing.List[str] = [] self.count = 0 for workload in ('container-a', 'containerb'): # Hook up relation events to generic handler. @@ -330,7 +332,7 @@ def __init__(self, *args): self.on[workload].pebble_ready, self.on_any_pebble_ready) - def on_any_pebble_ready(self, event): + def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): self.seen.append(type(event).__name__) self.count += 1 @@ -437,25 +439,25 @@ def test_action_events(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) framework.observe(self.on.start_action, self._on_start_action) - def _on_foo_bar_action(self, event): + def _on_foo_bar_action(self, event: ops.ActionEvent): self.seen_action_params = event.params event.log('test-log') event.set_results({'res': 'val with spaces'}) event.fail('test-fail') - def _on_start_action(self, event): + def _on_start_action(self, event: ops.ActionEvent): pass self._setup_test_action() framework = self.create_framework() charm = MyCharm(framework) - events = list(MyCharm.on.events()) + events: typing.List[str] = list(MyCharm.on.events()) # type: ignore self.assertIn('foo_bar_action', events) self.assertIn('start_action', events) @@ -477,12 +479,12 @@ def test_invalid_action_results(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) - self.res = {} + self.res: typing.Dict[str, typing.Any] = {} framework.observe(self.on.foo_bar_action, self._on_foo_bar_action) - def _on_foo_bar_action(self, event): + def _on_foo_bar_action(self, event: ops.ActionEvent): event.set_results(self.res) self._setup_test_action() @@ -500,15 +502,15 @@ def _on_foo_bar_action(self, event): with self.assertRaises(ValueError): charm.on.foo_bar_action.emit() - def _test_action_event_defer_fails(self, cmd_type): + def _test_action_event_defer_fails(self, cmd_type: str): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) framework.observe(self.on.start_action, self._on_start_action) - def _on_start_action(self, event): + def _on_start_action(self, event: ops.ActionEvent): event.defer() fake_script(self, f"{cmd_type}-get", """echo '{"foo-name": "name", "silent": true}'""") @@ -588,31 +590,31 @@ def test_containers_storage_multiple_mounts(self): def test_secret_events(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) - self.seen = [] + self.seen: typing.List[str] = [] self.framework.observe(self.on.secret_changed, self.on_secret_changed) self.framework.observe(self.on.secret_rotate, self.on_secret_rotate) self.framework.observe(self.on.secret_remove, self.on_secret_remove) self.framework.observe(self.on.secret_expired, self.on_secret_expired) - def on_secret_changed(self, event): + def on_secret_changed(self, event: ops.SecretChangedEvent): assert event.secret.id == 'secret:changed' assert event.secret.label is None self.seen.append(type(event).__name__) - def on_secret_rotate(self, event): + def on_secret_rotate(self, event: ops.SecretRotateEvent): assert event.secret.id == 'secret:rotate' assert event.secret.label == 'rot' self.seen.append(type(event).__name__) - def on_secret_remove(self, event): + def on_secret_remove(self, event: ops.SecretRemoveEvent): assert event.secret.id == 'secret:remove' assert event.secret.label == 'rem' assert event.revision == 7 self.seen.append(type(event).__name__) - def on_secret_expired(self, event): + def on_secret_expired(self, event: ops.SecretExpiredEvent): assert event.secret.id == 'secret:expired' assert event.secret.label == 'exp' assert event.revision == 42 @@ -635,11 +637,11 @@ def on_secret_expired(self, event): def test_collect_app_status_leader(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_app_status, self._on_collect_status) - def _on_collect_status(self, event): + def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status(ops.ActiveStatus()) event.add_status(ops.BlockedStatus('first')) event.add_status(ops.WaitingStatus('waiting')) @@ -658,11 +660,11 @@ def _on_collect_status(self, event): def test_collect_app_status_no_statuses(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_app_status, self._on_collect_status) - def _on_collect_status(self, event): + def _on_collect_status(self, event: ops.CollectStatusEvent): pass fake_script(self, 'is-leader', 'echo true') @@ -676,11 +678,11 @@ def _on_collect_status(self, event): def test_collect_app_status_non_leader(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_app_status, self._on_collect_status) - def _on_collect_status(self, event): + def _on_collect_status(self, event: ops.CollectStatusEvent): raise Exception # shouldn't be called fake_script(self, 'is-leader', 'echo false') @@ -694,11 +696,11 @@ def _on_collect_status(self, event): def test_collect_unit_status(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_unit_status, self._on_collect_status) - def _on_collect_status(self, event): + def _on_collect_status(self, event: ops.CollectStatusEvent): event.add_status(ops.ActiveStatus()) event.add_status(ops.BlockedStatus('first')) event.add_status(ops.WaitingStatus('waiting')) @@ -717,11 +719,11 @@ def _on_collect_status(self, event): def test_collect_unit_status_no_statuses(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_unit_status, self._on_collect_status) - def _on_collect_status(self, event): + def _on_collect_status(self, event: ops.CollectStatusEvent): pass fake_script(self, 'is-leader', 'echo false') # called only for collecting app statuses @@ -735,15 +737,15 @@ def _on_collect_status(self, event): def test_collect_app_and_unit_status(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_app_status, self._on_collect_app_status) self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status) - def _on_collect_app_status(self, event): + def _on_collect_app_status(self, event: ops.CollectStatusEvent): event.add_status(ops.ActiveStatus()) - def _on_collect_unit_status(self, event): + def _on_collect_unit_status(self, event: ops.CollectStatusEvent): event.add_status(ops.WaitingStatus('blah')) fake_script(self, 'is-leader', 'echo true') @@ -760,12 +762,12 @@ def _on_collect_unit_status(self, event): def test_add_status_type_error(self): class MyCharm(ops.CharmBase): - def __init__(self, *args): + def __init__(self, *args: typing.Any): super().__init__(*args) self.framework.observe(self.on.collect_app_status, self._on_collect_status) - def _on_collect_status(self, event): - event.add_status('active') + def _on_collect_status(self, event: ops.CollectStatusEvent): + event.add_status('active') # type: ignore fake_script(self, 'is-leader', 'echo true') @@ -775,12 +777,12 @@ def _on_collect_status(self, event): def test_collect_status_priority(self): class MyCharm(ops.CharmBase): - def __init__(self, *args, statuses=None): + def __init__(self, *args: typing.Any, statuses: typing.List[str]): super().__init__(*args) self.framework.observe(self.on.collect_app_status, self._on_collect_status) self.statuses = statuses - def _on_collect_status(self, event): + def _on_collect_status(self, event: ops.CollectStatusEvent): for status in self.statuses: event.add_status(ops.StatusBase.from_name(status, '')) From 718fb037ba3a4bafecbce606741e1328575f129c Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 2 Oct 2023 15:17:26 +1300 Subject: [PATCH 3/3] Fix last type hint issues. --- test/test_charm.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_charm.py b/test/test_charm.py index 71d12c149..e0b748ce6 100755 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -101,7 +101,8 @@ def test_observe_decorated_method(self): # way we know of to cleanly decorate charm event observers. events: typing.List[ops.EventBase] = [] - def dec(fn: typing.Callable[['MyCharm', ops.EventBase], None]): + def dec(fn: typing.Callable[['MyCharm', ops.EventBase], None] # noqa: F821 + ) -> typing.Callable[..., None]: # simple decorator that appends to the nonlocal # `events` list all events it receives @functools.wraps(fn)