Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 0 additions & 15 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -226,21 +226,6 @@ exclude = ["tracing/ops_tracing/vendor/*"]
"testing/tests/*" = [
# All documentation linting.
"D",
# TODO: the below ignores should be fixed
"CPY", # flake8-copyright
"I001", # isort
"B017", # Do not assert blind exception
"B018", # Useless attribute access
"E501", # Line too long
"N999", # Invalid module name
"N813", # CamelCase imported as lowercase
"S105", # Possible hardcoded password
"S108", # Probably insecure usage of /tmp
"W291", # Trailing whitespace
"UP037", # Remove quotes from type annotation
"RUF012", # Mutable class attributes should be annotated with `typing.ClassVar`
"RUF015", # Prefer `next` over single element slice
"SIM115", # Use a context manager for opening files
]
"ops/_private/timeconv.py" = [
"RUF001", # String contains ambiguous `µ` (MICRO SIGN). Did you mean `μ` (GREEK SMALL LETTER MU)?
Expand Down
2 changes: 2 additions & 0 deletions testing/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.
24 changes: 13 additions & 11 deletions testing/tests/helpers.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import dataclasses
import logging
from collections.abc import Callable
from pathlib import Path
from typing import TYPE_CHECKING, Any, TypeVar
from collections.abc import Callable

import jsonpatch

from scenario.context import _DEFAULT_JUJU_VERSION, Context
from scenario.state import _Event

Expand All @@ -20,17 +22,17 @@


def trigger(
state: 'State',
event: str | '_Event',
charm_type: type['CharmType'],
pre_event: Callable[['CharmType'], None] | None = None,
post_event: Callable[['CharmType'], None] | None = None,
state: State,
event: str | _Event,
charm_type: type[CharmType],
pre_event: Callable[[CharmType], None] | None = None,
post_event: Callable[[CharmType], None] | None = None,
meta: dict[str, Any] | None = None,
actions: dict[str, Any] | None = None,
config: dict[str, Any] | None = None,
charm_root: str | Path | None = None,
juju_version: str = _DEFAULT_JUJU_VERSION,
) -> 'State':
) -> State:
ctx = Context(
charm_type=charm_type,
meta=meta,
Expand All @@ -42,10 +44,10 @@ def trigger(
if isinstance(event, str):
if event.startswith('relation_'):
assert len(tuple(state.relations)) == 1, 'shortcut only works with one relation'
event = getattr(ctx.on, event)(tuple(state.relations)[0])
event = getattr(ctx.on, event)(next(iter(state.relations)))
elif event.startswith('pebble_'):
assert len(tuple(state.containers)) == 1, 'shortcut only works with one container'
event = getattr(ctx.on, event)(tuple(state.containers)[0])
event = getattr(ctx.on, event)(next(iter(state.containers)))
Comment on lines 44 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

These are improvements on their own, but feel weird given that we still call tuple(...) on the preceding line. Also unfortunate that we need to do len(tuple(... instead of just len( given that these are actually going to be frozenset.

I wonder if it would be worth adding custom __init__ methods to the relevant classes (just State?) to correct the mismatch between the relations arg only needing to be Iterable, but the relations attribute actually always being a frozenset -- should I create an issue for this? Or discuss at daily?

In the meantime, if we're open to making some manual improvements in this PR, how about this:

        if event.startswith('relation_'):
            (relation,) = state.relations  # shortcut only works with one relation
            event = getattr(ctx.on, event)(relation)
        elif event.startswith('pebble_'):
            (container,) = state.containers  # shortcut only works with one container
            event = getattr(ctx.on, event)(container)

But feel free to reject to minimise the diff.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if it would be worth adding custom __init__ methods to the relevant classes (just State?) to correct the mismatch between the relations arg only needing to be Iterable, but the relations attribute actually always being a frozenset -- should I create an issue for this? Or discuss at daily?

Yeah, this has come up a couple of times before.

We did actually have a version where they all had an __init__. The primary reason was to support keyword-only arguments in Python 3.8, and we eventually decided that having in one place (the thing with the custom __new__) was superior and ripped them all out. But they did also nicely handle the "you can give me an X but the attribute will always be a Y" case, so we lost that at the same time. Obviously, we have since ripped out the custom solution.

I'm definitely not opposed to going back to __init__. I think if we fix it for one it would be best to fix it for them all. Looking through, that's:

  • CloudCredential (Mapping->dict, Sequence->list, really not sure why that is not tuple!)
  • CloudSpec(Sequence->list, really not sure why that is not tuple!)
  • Secret(Mapping->RawSecretRevisionContents, could probably just be dict; Mapping->dict)
  • BindAddress(Sequence->tuple, at the moment stays a list if passed a list)

And probably lots more (I wouldn't be surprised if it's like 80% of the classes). However, it could also potentially get rid of or incorporate the code we have for doing the copy (for somewhat immutability).

Feel free to open a PR or add to daily.

else:
event = getattr(ctx.on, event)()
assert isinstance(event, _Event)
Expand All @@ -58,7 +60,7 @@ def trigger(
return state_out


def jsonpatch_delta(self, other: 'State'):
def jsonpatch_delta(self, other: State):
dict_other = dataclasses.asdict(other)
dict_self = dataclasses.asdict(self)
for attr in (
Expand Down
4 changes: 3 additions & 1 deletion testing/tests/test_charm_spec_autoload.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import importlib
Expand All @@ -7,7 +10,6 @@

import pytest
import yaml

from scenario import Context, Relation, State
from scenario.context import ContextSetupError
from scenario.state import CharmType, MetadataNotFoundError, _CharmSpec
Expand Down
19 changes: 11 additions & 8 deletions testing/tests/test_consistency_checker.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import dataclasses

import pytest
import ops

from scenario._consistency_checker import check_consistency
from scenario.context import Context
from scenario.errors import InconsistentScenarioError
Expand All @@ -29,15 +30,17 @@
_Event,
)

import ops


class MyCharm(ops.CharmBase):
pass


def assert_inconsistent(
state: 'State',
event: '_Event',
charm_spec: '_CharmSpec',
state: State,
event: _Event,
charm_spec: _CharmSpec,
juju_version='3.0',
unit_id=0,
):
Expand All @@ -46,9 +49,9 @@ def assert_inconsistent(


def assert_consistent(
state: 'State',
event: '_Event',
charm_spec: '_CharmSpec',
state: State,
event: _Event,
charm_spec: _CharmSpec,
juju_version='3.0',
unit_id=0,
):
Expand Down
7 changes: 5 additions & 2 deletions testing/tests/test_context.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import os
from unittest.mock import patch

import pytest
from ops import CharmBase

from scenario import Context, State
from scenario.errors import UncaughtCharmError
from scenario.state import _Event, _next_action_id

from ops import CharmBase


class MyCharm(CharmBase):
pass
Expand Down
8 changes: 5 additions & 3 deletions testing/tests/test_context_on.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import copy
import datetime
import typing

import ops
import pytest

import scenario

import ops

META = {
'name': 'context-charm',
Expand Down Expand Up @@ -333,7 +335,7 @@ def test_relation_departed_event():
relation = scenario.Relation('baz')
state_in = scenario.State(relations=[relation])
# These look like:
# ctx.run(ctx.on.baz_relation_departed(unit=unit_ordinal, departing_unit=unit_ordinal), state)
# ctx.run(ctx.on.baz_relation_departed(unit=unit_num, departing_unit=unit_num), state)
with ctx(ctx.on.relation_departed(relation, remote_unit=2, departing_unit=1), state_in) as mgr:
mgr.run()
relation_event, collect_status = mgr.charm.observed
Expand Down
2 changes: 2 additions & 0 deletions testing/tests/test_e2e/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.
7 changes: 5 additions & 2 deletions testing/tests/test_e2e/conftest.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

from typing import Any
from collections.abc import Generator
from typing import Any

import pytest
import yaml

from scenario import Context

from test.charms.test_secrets.src.charm import SecretsCharm


Expand Down
11 changes: 7 additions & 4 deletions testing/tests/test_e2e/test_actions.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import pytest
from scenario import Context
from scenario.state import State, _Action, _next_action_id

from ops import __version__ as ops_version
from ops._private.harness import ActionFailed
from ops.charm import ActionEvent, CharmBase
from ops.framework import Framework
from ops._private.harness import ActionFailed

from scenario import Context
from scenario.state import State, _Action, _next_action_id


@pytest.fixture(scope='function')
Expand Down
7 changes: 5 additions & 2 deletions testing/tests/test_e2e/test_cloud_spec.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import ops
import pytest

import scenario

import ops


class MyCharm(ops.CharmBase):
def __init__(self, framework: ops.Framework):
Expand Down
6 changes: 5 additions & 1 deletion testing/tests/test_e2e/test_config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import pytest
from scenario.state import State

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

from scenario.state import State
from ..helpers import trigger


Expand Down
15 changes: 10 additions & 5 deletions testing/tests/test_e2e/test_deferred.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import typing
from collections.abc import Mapping

import ops
import pytest
from ops.framework import LifecycleEvent

from scenario import Context
from scenario.state import Container, Relation, State, _Event

import ops
from ops.framework import LifecycleEvent

from ..helpers import trigger

CHARM_CALLED = 0
Expand All @@ -16,13 +21,13 @@
@pytest.fixture(scope='function')
def mycharm():
class MyCharm(ops.CharmBase):
META: dict[str, typing.Any] = {
META: Mapping[str, typing.Any] = {
'name': 'mycharm',
'requires': {'foo': {'interface': 'bar'}},
'containers': {'foo': {'type': 'oci-image'}},
}
defer_next = 0
captured: list[ops.EventBase] = []
captured: typing.ClassVar[list[ops.EventBase]] = []

def __init__(self, framework: ops.Framework):
super().__init__(framework)
Expand Down
21 changes: 9 additions & 12 deletions testing/tests/test_e2e/test_event.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from __future__ import annotations

import ops
import pytest
from ops import CharmBase

from scenario import Context
from scenario.state import State, _CharmSpec, _Event, _EventType

import ops


@pytest.mark.parametrize(
'evt, expected_type',
Expand Down Expand Up @@ -40,7 +42,7 @@ def test_event_type(evt, expected_type):
assert event._is_secret_event is (expected_type is _EventType.SECRET)
assert event._is_action_event is (expected_type is _EventType.ACTION)

class MyCharm(CharmBase):
class MyCharm(ops.CharmBase):
pass

spec = _CharmSpec(
Expand All @@ -61,10 +63,7 @@ class MyCharm(CharmBase):


def test_emitted_framework():
class MyCharm(CharmBase):
META = {'name': 'joop'}

ctx = Context(MyCharm, meta=MyCharm.META, capture_framework_events=True)
ctx = Context(ops.CharmBase, meta={'name': 'joop'}, capture_framework_events=True)
ctx.run(ctx.on.update_status(), State())
assert len(ctx.emitted_events) == 4
assert list(map(type, ctx.emitted_events)) == [
Expand All @@ -76,9 +75,7 @@ class MyCharm(CharmBase):


def test_emitted_deferred():
class MyCharm(CharmBase):
META = {'name': 'joop'}

class MyCharm(ops.CharmBase):
def __init__(self, framework: ops.Framework):
super().__init__(framework)
framework.observe(self.on.update_status, self._on_update_status)
Expand All @@ -88,7 +85,7 @@ def _on_update_status(self, _: ops.UpdateStatusEvent):

ctx = Context(
MyCharm,
meta=MyCharm.META,
meta={'name': 'joop'},
capture_deferred_events=True,
capture_framework_events=True,
)
Expand Down
Loading
Loading