From 02d59be041d16531d911427f53091317947268e0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 12:26:18 +1200 Subject: [PATCH] Post-merge fixes. --- scenario/consistency_checker.py | 66 +++++++++---------------------- scenario/context.py | 5 --- scenario/mocking.py | 13 +----- scenario/state.py | 13 +++--- tests/test_consistency_checker.py | 3 +- tests/test_e2e/test_pebble.py | 4 +- tests/test_e2e/test_state.py | 4 +- 7 files changed, 32 insertions(+), 76 deletions(-) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index d8a233aa..dcb46652 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -193,26 +193,27 @@ def _check_workload_event( "cannot construct a workload event without the container instance. " "Please pass one.", ) - elif not event.name.startswith(normalize_name(event.container.name)): - errors.append( - f"workload event should start with container name. {event.name} does " - f"not start with {event.container.name}.", - ) - if event.container not in state.containers: + else: + if not event.name.startswith(normalize_name(event.container.name)): errors.append( - f"cannot emit {event.name} because container {event.container.name} " - f"is not in the state.", + f"workload event should start with container name. {event.name} does " + f"not start with {event.container.name}.", ) - if not event.container.can_connect: - warnings.append( - "you **can** fire fire pebble-ready while the container cannot connect, " - "but that's most likely not what you want.", + if event.container not in state.containers: + errors.append( + f"cannot emit {event.name} because container {event.container.name} " + f"is not in the state.", + ) + if not event.container.can_connect: + warnings.append( + "you **can** fire fire pebble-ready while the container cannot connect, " + "but that's most likely not what you want.", + ) + names = Counter(exec.command_prefix for exec in event.container.execs) + if dupes := [n for n in names if names[n] > 1]: + errors.append( + f"container {event.container.name} has duplicate command prefixes: {dupes}", ) - names = Counter(exec.command_prefix for exec in event.container.execs) - if dupes := [n for n in names if names[n] > 1]: - errors.append( - f"container {event.container.name} has duplicate command prefixes: {dupes}", - ) def _check_action_event( @@ -663,34 +664,3 @@ def check_storedstate_consistency( f"The StoredState object {ss.owner_path}.{ss.name} should contain only simple types.", ) return Results(errors, []) - - -def check_storedstate_consistency( - *, - state: "State", - **_kwargs, # noqa: U101 -) -> Results: - """Check the internal consistency of `state.storedstate`.""" - errors = [] - - # Attribute names must be unique on each object. - names = defaultdict(list) - for ss in state.stored_state: - names[ss.owner_path].append(ss.name) - for owner, owner_names in names.items(): - if len(owner_names) != len(set(owner_names)): - errors.append( - f"{owner} has multiple StoredState objects with the same name.", - ) - - # The content must be marshallable. - for ss in state.stored_state: - # We don't need the marshalled state, just to know that it can be. - # This is the same "only simple types" check that ops does. - try: - marshal.dumps(ss.content) - except ValueError: - errors.append( - f"The StoredState object {ss.owner_path}.{ss.name} should contain only simple types.", - ) - return Results(errors, []) diff --git a/scenario/context.py b/scenario/context.py index 20451ccb..6dcf910a 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -682,11 +682,6 @@ def _finalize_action(self, state_out: "State"): return ao - @contextmanager - def _run_action(self, action: "Action", state: "State"): - with self._run(event=action.event, state=state) as ops: - yield ops - @contextmanager def _run(self, event: "_Event", state: "State"): runtime = Runtime( diff --git a/scenario/mocking.py b/scenario/mocking.py index 91285075..6221baf3 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -1,7 +1,6 @@ #!/usr/bin/env python3 # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -import dataclasses import datetime import random import shutil @@ -79,7 +78,6 @@ def __init__( command: Tuple[str, ...], change_id: int, exec: "Exec", - container: "ContainerSpec", ): self._command = command self._change_id = change_id @@ -89,17 +87,9 @@ def __init__( self.stderr = StringIO(self._exec.stderr) # You can't pass *in* the stdin, the charm is responsible for that. self.stdin = StringIO() - self._container = container def _store_stdin(self): - execs = { - e - for e in self._container.execs - if e.command_prefix != self._exec.command_prefix - } - exec = dataclasses.replace(self._exec, stdin=self.stdin.read()) - execs.add(exec) - self._container._update_execs(execs) + self._exec._update_stdin(self.stdin.getvalue()) def wait(self): self._waited = True @@ -786,7 +776,6 @@ def exec(self, command, **kwargs): # noqa: U100 type: ignore change_id=change_id, command=command, exec=handler, - container=self._container, ) def _check_connection(self): diff --git a/scenario/state.py b/scenario/state.py index 77597808..03365cf4 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -644,8 +644,9 @@ def _generate_new_change_id(): @dataclasses.dataclass(frozen=True) -class Exec(_max_posargs(0)): +class Exec(_max_posargs(1)): """Mock data for simulated :meth:`ops.Container.exec` calls.""" + command_prefix: Sequence[str] return_code: int = 0 """The return code of the process (0 is success).""" @@ -662,6 +663,10 @@ class Exec(_max_posargs(0)): def _run(self) -> int: return self._change_id + def _update_stdin(self, stdin: str): + # bypass frozen dataclass + object.__setattr__(self, "stdin", stdin) + @dataclasses.dataclass(frozen=True) class Mount(_max_posargs(0)): @@ -828,7 +833,7 @@ class Container(_max_posargs(1)): } """ - execs: FrozenSet[Exec] = frozenset() + execs: Iterable[Exec] = frozenset() """Simulate executing commands in the container. Specify each command the charm might run in the container and a :class:`Exec` @@ -927,10 +932,6 @@ def get_exec(self, command_prefix: Sequence[str]): return exec raise KeyError(f"no exec found with command prefix {command_prefix}") - def _update_execs(self, execs: Iterable[Exec]): - # bypass frozen dataclass - object.__setattr__(self, "execs", frozenset(execs)) - _RawStatusLiteral = Literal[ "waiting", diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 8b52120b..769c84e5 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -12,8 +12,8 @@ CloudCredential, CloudSpec, Container, - Model, Exec, + Model, Network, Notice, PeerRelation, @@ -360,6 +360,7 @@ def test_secret_not_in_state(): _CharmSpec(MyCharm, {}), ) + def test_peer_relation_consistency(): assert_inconsistent( State(relations={Relation("foo")}), diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index a7e00e80..fa50e7a7 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -227,9 +227,9 @@ def _on_ready(self, _): exec = Exec((), stdout=LS) container = Container(name="foo", can_connect=True, execs={exec}) state_out = ctx.run( - ctx.on.pebble_ready(container=container), State(containers=[container]) + ctx.on.pebble_ready(container=container), State(containers={container}) ) - assert state_out.get_container(container).get_exec(()).stdin == "hello world!" + assert state_out.get_container(container.name).get_exec(()).stdin == "hello world!" def test_pebble_ready(charm_cls): diff --git a/tests/test_e2e/test_state.py b/tests/test_e2e/test_state.py index 325cda66..d6e3aa5c 100644 --- a/tests/test_e2e/test_state.py +++ b/tests/test_e2e/test_state.py @@ -279,9 +279,9 @@ def test_container_default_values(): assert container.name == name assert container.can_connect is False assert container.layers == {} - assert container.service_status == {} + assert container.service_statuses == {} assert container.mounts == {} - assert container.exec_mock == {} + assert container.execs == frozenset() assert container.layers == {} assert container._base_plan == {}