Skip to content

test: Extend the type checking of the ops-scenario tests #2234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tonyandrewmeyer merged 24 commits into from
Apr 13, 2026
Merged

test: Extend the type checking of the ops-scenario tests #2234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9a4bab3
test: add type annotations for test_network
tonyandrewmeyer Dec 12, 2025
0f3eb00
test: add type annotations to test_cloud_spec.
tonyandrewmeyer Dec 12, 2025
14b2923
test: add type annotations to test_play_assertions.
tonyandrewmeyer Dec 12, 2025
34adbbd
test: add type annotations to test_vroot.
tonyandrewmeyer Dec 12, 2025
8350e8d
test: add type annotations for test_resource.
tonyandrewmeyer Dec 12, 2025
0a8c1d5
test: add type annotations to test_state.
tonyandrewmeyer Dec 12, 2025
63c039a
test: add type annotations to test_actions.
tonyandrewmeyer Dec 12, 2025
58e19b0
test: add type annotations to test_config.
tonyandrewmeyer Dec 12, 2025
d82d82a
test: add type annotations to test_event.
tonyandrewmeyer Dec 12, 2025
eceb259
test: add type annotations to test_deferred.
tonyandrewmeyer Dec 13, 2025
49893fc
test: add tests for test_stored_state.
tonyandrewmeyer Dec 13, 2025
86ff536
chore: copy over the exclusions done in the other branch
tonyandrewmeyer Dec 13, 2025
eab3f63
test: add return annotations (this will clash with the other PR, we'l…
tonyandrewmeyer Dec 13, 2025
5f62371
chore: remove -> None.
tonyandrewmeyer Dec 15, 2025
c43500e
chore: remove unnecessary quotes.
tonyandrewmeyer Dec 15, 2025
f45fcb8
chore: remove scope from type: ignore
tonyandrewmeyer Dec 15, 2025
c070335
Merge origin/main.
tonyandrewmeyer Dec 16, 2025
b95c0c7
Post merge fixes.
tonyandrewmeyer Dec 16, 2025
85b2b0f
Merge remote-tracking branch 'origin/main' into type-check-scenario-t…
tonyandrewmeyer Mar 26, 2026
efc5473
Fix bad merging that added exclusions back.
tonyandrewmeyer Mar 26, 2026
95f9ada
Apply suggestion from @james-garner-canonical
tonyandrewmeyer Mar 30, 2026
3397086
Remove accidentally restored code.
tonyandrewmeyer Mar 30, 2026
8cf2cac
Remove accidentally restored code.
tonyandrewmeyer Mar 30, 2026
0edbff3
tox -e format
tonyandrewmeyer Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 30 additions & 28 deletions testing/tests/test_e2e/test_actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

from __future__ import annotations

from typing import Any

import pytest
from scenario import Context
from scenario.state import State, _Action, _next_action_id
Expand All @@ -13,7 +15,7 @@


@pytest.fixture(scope='function')
def mycharm():
def mycharm() -> type[CharmBase]:
class MyCharm(CharmBase):
_evt_handler = None

Expand All @@ -22,16 +24,16 @@ def __init__(self, framework: Framework):
for evt in self.on.events().values():
self.framework.observe(evt, self._on_event)

def _on_event(self, event):
def _on_event(self, event: ActionEvent):
if handler := self._evt_handler:
handler(event)

return MyCharm


@pytest.mark.parametrize('baz_value', (True, False))
def test_action_event(mycharm, baz_value):
ctx = Context(
def test_action_event(mycharm: type[CharmBase], baz_value: bool):
ctx: Context[CharmBase] = Context(
mycharm,
meta={'name': 'foo'},
actions={'foo': {'params': {'bar': {'type': 'number'}, 'baz': {'type': 'boolean'}}}},
Expand All @@ -47,11 +49,11 @@ def test_action_event(mycharm, baz_value):

def test_action_no_results():
class MyCharm(CharmBase):
def __init__(self, framework):
def __init__(self, framework: Framework):
super().__init__(framework)
framework.observe(self.on.act_action, self._on_act_action)

def _on_act_action(self, _):
def _on_act_action(self, _: ActionEvent):
pass

ctx = Context(MyCharm, meta={'name': 'foo'}, actions={'act': {}})
Expand All @@ -61,27 +63,27 @@ def _on_act_action(self, _):


@pytest.mark.parametrize('res_value', ('one', 1, [2], ['bar'], (1,), {1, 2}))
def test_action_event_results_invalid(mycharm, res_value):
def test_action_event_results_invalid(mycharm: type[CharmBase], res_value: object):
def handle_evt(charm: CharmBase, evt: ActionEvent):
with pytest.raises((TypeError, AttributeError)):
evt.set_results(res_value)
evt.set_results(res_value) # type: ignore

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore

ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})
ctx.run(ctx.on.action('foo'), State())


@pytest.mark.parametrize('res_value', ({'a': {'b': {'c'}}}, {'d': 'e'}))
def test_action_event_results_valid(mycharm, res_value):
def handle_evt(_: CharmBase, evt):
def test_action_event_results_valid(mycharm: type[CharmBase], res_value: dict[str, Any]):
def handle_evt(_: CharmBase, evt: ActionEvent):
if not isinstance(evt, ActionEvent):
return
evt.set_results(res_value)
evt.log('foo')
evt.log('bar')

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore

ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})

Expand All @@ -91,7 +93,7 @@ def handle_evt(_: CharmBase, evt):


@pytest.mark.parametrize('res_value', ({'a': {'b': {'c'}}}, {'d': 'e'}))
def test_action_event_outputs(mycharm, res_value):
def test_action_event_outputs(mycharm: type[CharmBase], res_value: dict[str, Any]):
def handle_evt(_: CharmBase, evt: ActionEvent):
if not isinstance(evt, ActionEvent):
return
Expand All @@ -101,7 +103,7 @@ def handle_evt(_: CharmBase, evt: ActionEvent):
evt.log('log2')
evt.fail('failed becozz')

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore

ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})
with pytest.raises(ActionFailed) as exc_info:
Expand All @@ -111,27 +113,27 @@ def handle_evt(_: CharmBase, evt: ActionEvent):
assert ctx.action_logs == ['log1', 'log2']


def test_action_event_fail(mycharm):
def test_action_event_fail(mycharm: type[CharmBase]):
def handle_evt(_: CharmBase, evt: ActionEvent):
if not isinstance(evt, ActionEvent):
return
evt.fail('action failed!')

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore
Copy link
Copy Markdown
Contributor

@dimaqq dimaqq Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not to happy about these type: ignore all over but I guess the change is an improvement overall and silencing can be removed later.


ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})
with pytest.raises(ActionFailed) as exc_info:
ctx.run(ctx.on.action('foo'), State())
assert exc_info.value.message == 'action failed!'


def test_action_event_fail_context_manager(mycharm):
def test_action_event_fail_context_manager(mycharm: type[CharmBase]):
def handle_evt(_: CharmBase, evt: ActionEvent):
if not isinstance(evt, ActionEvent):
return
evt.fail('action failed!')

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore

ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})
with pytest.raises(ActionFailed) as exc_info:
Expand All @@ -142,11 +144,11 @@ def handle_evt(_: CharmBase, evt: ActionEvent):

def test_action_continues_after_fail():
class MyCharm(CharmBase):
def __init__(self, framework):
def __init__(self, framework: Framework):
super().__init__(framework)
framework.observe(self.on.foo_action, self._on_foo_action)

def _on_foo_action(self, event):
def _on_foo_action(self, event: ActionEvent):
event.log('starting')
event.set_results({'initial': 'result'})
event.fail('oh no!')
Expand All @@ -160,44 +162,44 @@ def _on_foo_action(self, event):
assert ctx.action_results == {'initial': 'result', 'final': 'result'}


def test_action_event_has_id(mycharm):
def test_action_event_has_id(mycharm: type[CharmBase]):
def handle_evt(_: CharmBase, evt: ActionEvent):
if not isinstance(evt, ActionEvent):
return
assert isinstance(evt.id, str) and evt.id != ''

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore

ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})
ctx.run(ctx.on.action('foo'), State())


def test_action_event_has_override_id(mycharm):
def test_action_event_has_override_id(mycharm: type[CharmBase]):
uuid = '0ddba11-cafe-ba1d-5a1e-dec0debad'

def handle_evt(charm: CharmBase, evt: ActionEvent):
if not isinstance(evt, ActionEvent):
return
assert evt.id == uuid

mycharm._evt_handler = handle_evt
mycharm._evt_handler = handle_evt # type: ignore
Copy link
Copy Markdown
Contributor

@dimaqq dimaqq Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I drop something like this at the top of the file:

  class MyCharm(CharmBase):
      _evt_handler = Callable[[CharmBase, ActionEvent], None]

And update the test argument to mycharm: type[MyCharm], then I don't need # type: ignore here and all the other places.

WDYT about that, or perhaps a Protocol?

Copy link
Copy Markdown
Contributor

@james-garner-canonical james-garner-canonical Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you disagree with this comment? I think we'd get value out of being able to turn on static type checking for these files now, and deferring further iteration til later. pyright will help us get rid of type: ignore comments when we eventually improve type visibility (by complaining about them once they're unnecessary).


ctx = Context(mycharm, meta={'name': 'foo'}, actions={'foo': {}})
ctx.run(ctx.on.action('foo', id=uuid), State())


def test_two_actions_same_context():
class MyCharm(CharmBase):
def __init__(self, framework):
def __init__(self, framework: Framework):
super().__init__(framework)
framework.observe(self.on.foo_action, self._on_foo_action)
framework.observe(self.on.bar_action, self._on_bar_action)

def _on_foo_action(self, event):
def _on_foo_action(self, event: ActionEvent):
event.log('foo')
event.set_results({'foo': 'result'})

def _on_bar_action(self, event):
def _on_bar_action(self, event: ActionEvent):
event.log('bar')
event.set_results({'bar': 'result'})

Expand All @@ -213,7 +215,7 @@ def _on_bar_action(self, event):

def test_positional_arguments():
with pytest.raises(TypeError):
_Action('foo', {})
_Action('foo', {}) # type: ignore


def test_default_arguments():
Expand Down
2 changes: 1 addition & 1 deletion testing/tests/test_e2e/test_cloud_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def __init__(self, framework: ops.Framework):
for evt in self.on.events().values():
self.framework.observe(evt, self._on_event)

def _on_event(self, event):
def _on_event(self, event: ops.EventBase):
pass


Expand Down
35 changes: 18 additions & 17 deletions testing/tests/test_e2e/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,32 @@

from __future__ import annotations

from typing import Any

import pytest
from scenario.state import State

from ops.charm import CharmBase
from ops.framework import Framework
import ops

from ..helpers import trigger


@pytest.fixture(scope='function')
def mycharm():
class MyCharm(CharmBase):
def __init__(self, framework: Framework):
def mycharm() -> type[ops.CharmBase]:
class MyCharm(ops.CharmBase):
def __init__(self, framework: ops.Framework):
super().__init__(framework)
for evt in self.on.events().values():
self.framework.observe(evt, self._on_event)

def _on_event(self, event):
def _on_event(self, event: ops.EventBase):
pass

return MyCharm


def test_config_get(mycharm):
def check_cfg(charm: CharmBase):
def test_config_get(mycharm: type[ops.CharmBase]):
def check_cfg(charm: ops.CharmBase):
assert charm.config['foo'] == 'bar'
assert charm.config['baz'] == 1

Expand All @@ -43,8 +44,8 @@ def check_cfg(charm: CharmBase):
)


def test_config_get_default_from_meta(mycharm):
def check_cfg(charm: CharmBase):
def test_config_get_default_from_meta(mycharm: type[ops.CharmBase]):
def check_cfg(charm: ops.CharmBase):
assert charm.config['foo'] == 'bar'
assert charm.config['baz'] == 2
assert charm.config['qux'] is False
Expand Down Expand Up @@ -75,18 +76,18 @@ def check_cfg(charm: CharmBase):
{'baz': 4, 'foo': 'bar', 'qux': True},
),
)
def test_config_in_not_mutated(mycharm, cfg_in):
class MyCharm(CharmBase):
def __init__(self, framework: Framework):
def test_config_in_not_mutated(mycharm: type[ops.CharmBase], cfg_in: dict[str, Any]):
class MyCharm(ops.CharmBase):
def __init__(self, framework: ops.Framework):
super().__init__(framework)
for evt in self.on.events().values():
self.framework.observe(evt, self._on_event)

def _on_event(self, event):
def _on_event(self, event: ops.EventBase):
# access the config to trigger a config-get
foo_cfg = self.config['foo'] # noqa: F841
baz_cfg = self.config['baz'] # noqa: F841
qux_cfg = self.config['qux'] # noqa: F841
_foo_cfg = self.config['foo']
Comment thread
tonyandrewmeyer marked this conversation as resolved.
_baz_cfg = self.config['baz']
Comment thread
tonyandrewmeyer marked this conversation as resolved.
_qux_cfg = self.config['qux']
Comment thread
tonyandrewmeyer marked this conversation as resolved.

state_out = trigger(
State(
Expand Down
Loading