From 73eba4f66e9ddf0bd525f6d744715863442ec52e Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 18 Apr 2024 23:23:40 +1200 Subject: [PATCH 01/40] Rename and extend container execs. --- README.md | 13 ++++--- scenario/__init__.py | 4 +-- scenario/mocking.py | 66 +++++++++++++++++++++++++++++------ scenario/state.py | 20 +++++------ tests/test_e2e/test_pebble.py | 24 ++++++++----- 5 files changed, 91 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 3052ecf6..aed56c42 100644 --- a/README.md +++ b/README.md @@ -639,17 +639,19 @@ class MyCharm(ops.CharmBase): def _on_start(self, _): foo = self.unit.get_container('foo') proc = foo.exec(['ls', '-ll']) - stdout, _ = proc.wait_output() + stdout, _ = proc.wait_output("...") assert stdout == LS_LL def test_pebble_exec(): container = scenario.Container( name='foo', - exec_mock={ - ('ls', '-ll'): # this is the command we're mocking - scenario.ExecOutput(return_code=0, # this data structure contains all we need to mock the call. - stdout=LS_LL) + execs={ + "list-directory": scenario.Exec( # "list-directory" is an arbitrary tag for the command + ('ls', '-ll'): # this is the command we're mocking + return_code=0, + stdout=LS_LL + ), } ) state_in = scenario.State(containers={container}) @@ -661,6 +663,7 @@ def test_pebble_exec(): ctx.on.pebble_ready(container), state_in, ) + assert state_out.containers["foo"].execs["list-directory"].stdin = "..." ``` ### Pebble Notices diff --git a/scenario/__init__.py b/scenario/__init__.py index bbd2c694..40a7b172 100644 --- a/scenario/__init__.py +++ b/scenario/__init__.py @@ -13,7 +13,7 @@ Container, DeferredEvent, ErrorStatus, - ExecOutput, + Exec, ICMPPort, MaintenanceStatus, Model, @@ -49,7 +49,7 @@ "SubordinateRelation", "PeerRelation", "Model", - "ExecOutput", + "Exec", "Mount", "Container", "Notice", diff --git a/scenario/mocking.py b/scenario/mocking.py index 396c4619..3606a67c 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +import dataclasses import datetime import random import shutil @@ -53,7 +54,7 @@ from scenario.context import Context from scenario.state import Container as ContainerSpec from scenario.state import ( - ExecOutput, + Exec, Relation, Secret, State, @@ -73,25 +74,53 @@ class ActionMissingFromContextError(Exception): class _MockExecProcess: - def __init__(self, command: Tuple[str, ...], change_id: int, out: "ExecOutput"): + def __init__( + self, + command: Tuple[str, ...], + change_id: int, + out: "Exec", + container: "ContainerSpec", + exec_id: str, + ): self._command = command self._change_id = change_id self._out = out self._waited = False self.stdout = StringIO(self._out.stdout) self.stderr = StringIO(self._out.stderr) + self.stdin = StringIO(self._out.stdin) + self._container = container + self._exec_id = exec_id def wait(self): self._waited = True + self._container.execs[self._exec_id] = dataclasses.replace( + self._container.execs[self._exec_id], + stdin=self.stdin.read(), + ) exit_code = self._out.return_code if exit_code != 0: - raise ExecError(list(self._command), exit_code, None, None) + raise ExecError( + list(self._command), + exit_code, + self.stdout.read(), + self.stderr.read(), + ) def wait_output(self): out = self._out + self._container.execs[self._exec_id] = dataclasses.replace( + self._container.execs[self._exec_id], + stdin=self.stdin.read(), + ) exit_code = out.return_code if exit_code != 0: - raise ExecError(list(self._command), exit_code, None, None) + raise ExecError( + list(self._command), + exit_code, + self.stdout.read(), + self.stderr.read(), + ) return out.stdout, out.stderr def send_signal(self, sig: Union[int, str]): # noqa: U100 @@ -729,19 +758,36 @@ def _layers(self) -> Dict[str, pebble.Layer]: def _service_status(self) -> Dict[str, pebble.ServiceStatus]: return self._container.service_status - def exec(self, *args, **kwargs): # noqa: U100 type: ignore - cmd = tuple(args[0]) - out = self._container.exec_mock.get(cmd) + # Based on a method of the same name from ops.testing. + def _find_exec_handler(self, command) -> Tuple[Optional[str], Optional["Exec"]]: + handlers = { + tuple(exc.command): (key, exc) for key, exc in self._container.execs.items() + } + for prefix_len in reversed(range(len(command) + 1)): + command_prefix = tuple(command[:prefix_len]) + if command_prefix in handlers: + return handlers[command_prefix] + return None, None + + def exec(self, command, **kwargs): # noqa: U100 type: ignore + exec_id, out = self._find_exec_handler(command) if not out: raise RuntimeError( - f"mock for cmd {cmd} not found. Please pass to the Container " - f"{self._container.name} a scenario.ExecOutput mock for the " + f"mock for cmd {command} not found. Please pass to the Container " + f"{self._container.name} a scenario.Exec mock for the " f"command your charm is attempting to run, or patch " f"out whatever leads to the call.", ) + assert exec_id is not None # For the static checker. change_id = out._run() - return _MockExecProcess(change_id=change_id, command=cmd, out=out) + return _MockExecProcess( + change_id=change_id, + command=command, + out=out, + container=self._container, + exec_id=exec_id, + ) def _check_connection(self): if not self._container.can_connect: diff --git a/scenario/state.py b/scenario/state.py index 6101e7cb..2345ca10 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -23,6 +23,7 @@ List, Literal, Optional, + Sequence, Set, Tuple, Type, @@ -643,15 +644,17 @@ def _generate_new_change_id(): @dataclasses.dataclass(frozen=True) -class ExecOutput(_max_posargs(0)): +class Exec(_max_posargs(0)): """Mock data for simulated :meth:`ops.Container.exec` calls.""" - + command: Sequence[str] return_code: int = 0 """The return code of the process (0 is success).""" stdout: str = "" """Any content written to stdout by the process.""" stderr: str = "" """Any content written to stderr by the process.""" + stdin: Optional[str] = None + """Any content written to stdin by the charm.""" # change ID: used internally to keep track of mocked processes _change_id: int = dataclasses.field(default_factory=_generate_new_change_id) @@ -660,9 +663,6 @@ def _run(self) -> int: return self._change_id -_ExecMock = Dict[Tuple[str, ...], ExecOutput] - - @dataclasses.dataclass(frozen=True) class Mount(_max_posargs(0)): """Maps local files to a :class:`Container` filesystem.""" @@ -828,20 +828,20 @@ class Container(_max_posargs(1)): } """ - exec_mock: _ExecMock = dataclasses.field(default_factory=dict) + execs: Dict[str, Exec] = dataclasses.field(default_factory=dict) """Simulate executing commands in the container. - Specify each command the charm might run in the container and a :class:`ExecOutput` + Specify each command the charm might run in the container and a :class:`Exec` containing its return code and any stdout/stderr. For example:: container = scenario.Container( name='foo', - exec_mock={ - ('whoami', ): scenario.ExecOutput(return_code=0, stdout='ubuntu') + execs={ + ('whoami', ): scenario.Exec(return_code=0, stdout='ubuntu') ('dig', '+short', 'canonical.com'): - scenario.ExecOutput(return_code=0, stdout='185.125.190.20\\n185.125.190.21') + scenario.Exec(return_code=0, stdout='185.125.190.20\\n185.125.190.21') } ) """ diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index da40cf5d..30e643aa 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -10,7 +10,7 @@ from ops.pebble import ExecError, ServiceStartup, ServiceStatus from scenario import Context -from scenario.state import CheckInfo, Container, ExecOutput, Mount, Notice, State +from scenario.state import CheckInfo, Container, Exec, Mount, Notice, State from tests.helpers import jsonpatch_delta, trigger @@ -193,7 +193,7 @@ def callback(self: CharmBase): container = self.unit.get_container("foo") proc = container.exec([cmd]) proc.wait() - assert proc.stdout.read() == "hello pebble" + assert proc.stdout.read() == out trigger( State( @@ -201,7 +201,9 @@ def callback(self: CharmBase): Container( name="foo", can_connect=True, - exec_mock={(cmd,): ExecOutput(stdout="hello pebble")}, + execs={ + "cmd": Exec([cmd], stdout=out), + }, ) } ), @@ -312,7 +314,9 @@ def test_exec_wait_error(charm_cls): Container( name="foo", can_connect=True, - exec_mock={("foo",): ExecOutput(stdout="hello pebble", return_code=1)}, + execs={ + "error": Exec(("foo",), stdout="hello pebble", return_code=1), + }, ) } ) @@ -321,9 +325,9 @@ def test_exec_wait_error(charm_cls): with ctx.manager(ctx.on.start(), state) as mgr: container = mgr.charm.unit.get_container("foo") proc = container.exec(["foo"]) - with pytest.raises(ExecError): + with pytest.raises(ExecError) as exc_info: proc.wait() - assert proc.stdout.read() == "hello pebble" + assert exc_info.value.stdout == "hello pebble" def test_exec_wait_output(charm_cls): @@ -332,8 +336,8 @@ def test_exec_wait_output(charm_cls): Container( name="foo", can_connect=True, - exec_mock={ - ("foo",): ExecOutput(stdout="hello pebble", stderr="oepsie") + execs={ + "waiter": Exec(("foo",), stdout="hello pebble", stderr="oepsie") }, ) } @@ -354,7 +358,9 @@ def test_exec_wait_output_error(charm_cls): Container( name="foo", can_connect=True, - exec_mock={("foo",): ExecOutput(stdout="hello pebble", return_code=1)}, + execs={ + "error": Exec(("foo",), stdout="hello pebble", return_code=1), + }, ) } ) From 63b62c86e546c1a1ac7585dad1d7882509eef685 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 23 Apr 2024 10:53:15 +1200 Subject: [PATCH 02/40] wait() doesn't have the stdout/stderr in the exception (it's not available at all). --- scenario/mocking.py | 4 ++-- tests/test_e2e/test_pebble.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 3606a67c..16ae92dd 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -103,8 +103,8 @@ def wait(self): raise ExecError( list(self._command), exit_code, - self.stdout.read(), - self.stderr.read(), + None, + None, ) def wait_output(self): diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 30e643aa..4474d0ea 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -326,7 +326,7 @@ def test_exec_wait_error(charm_cls): container = mgr.charm.unit.get_container("foo") proc = container.exec(["foo"]) with pytest.raises(ExecError) as exc_info: - proc.wait() + proc.wait_output() assert exc_info.value.stdout == "hello pebble" From 830c732e32e6a711ab5bb70694f8799d0516e4eb Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 2 Apr 2024 11:44:31 +1300 Subject: [PATCH 03/40] Add basic StoredState consistency checks. --- scenario/consistency_checker.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 1334bbbd..61d34b16 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -658,3 +658,34 @@ 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"{ss!r} must contain only simple types.", + ) + return Results(errors, []) From e84a1c3ccee45fa98bd93ca7d79e8be483fdddde Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 24 May 2024 12:41:56 +1200 Subject: [PATCH 04/40] Update scenario/consistency_checker.py Co-authored-by: PietroPasotti --- scenario/consistency_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 61d34b16..e16fb300 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -686,6 +686,6 @@ def check_storedstate_consistency( marshal.dumps(ss.content) except ValueError: errors.append( - f"{ss!r} must contain only simple types.", + f"The StoredState object {ss.owner_path}.{ss.name} should contain only simple types.", ) return Results(errors, []) From e827b924dd2443e2f7c2f1b9fd8ebb29009455be Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 28 May 2024 17:27:18 +1200 Subject: [PATCH 05/40] Update scenario/mocking.py Co-authored-by: PietroPasotti --- scenario/mocking.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 16ae92dd..c8e25812 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -761,7 +761,7 @@ def _service_status(self) -> Dict[str, pebble.ServiceStatus]: # Based on a method of the same name from ops.testing. def _find_exec_handler(self, command) -> Tuple[Optional[str], Optional["Exec"]]: handlers = { - tuple(exc.command): (key, exc) for key, exc in self._container.execs.items() + tuple(exec.command): (exec_id, exec) for exec_id, exec in self._container.execs.items() } for prefix_len in reversed(range(len(command) + 1)): command_prefix = tuple(command[:prefix_len]) From 01ae7acba8a08aec0ba3cb532b35f88694ced7e7 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 18:56:21 +1200 Subject: [PATCH 06/40] Make container.execs a frozenset. --- README.md | 6 ++-- scenario/mocking.py | 58 +++++++++++++++++------------------ scenario/state.py | 15 +++++++-- tests/test_e2e/test_pebble.py | 10 +++--- 4 files changed, 48 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index aed56c42..a7a3cd22 100644 --- a/README.md +++ b/README.md @@ -647,8 +647,8 @@ def test_pebble_exec(): container = scenario.Container( name='foo', execs={ - "list-directory": scenario.Exec( # "list-directory" is an arbitrary tag for the command - ('ls', '-ll'): # this is the command we're mocking + scenario.Exec( + ('ls', '-ll'), # this is the command we're mocking return_code=0, stdout=LS_LL ), @@ -663,7 +663,7 @@ def test_pebble_exec(): ctx.on.pebble_ready(container), state_in, ) - assert state_out.containers["foo"].execs["list-directory"].stdin = "..." + assert state_out.containers["foo"].get_exec(('ls', '-ll')).stdin = "..." ``` ### Pebble Notices diff --git a/scenario/mocking.py b/scenario/mocking.py index c8e25812..4d6faf76 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -78,27 +78,32 @@ def __init__( self, command: Tuple[str, ...], change_id: int, - out: "Exec", + exec: "Exec", container: "ContainerSpec", - exec_id: str, ): self._command = command self._change_id = change_id - self._out = out + self._exec = exec self._waited = False - self.stdout = StringIO(self._out.stdout) - self.stderr = StringIO(self._out.stderr) - self.stdin = StringIO(self._out.stdin) + self.stdout = StringIO(self._exec.stdout) + self.stderr = StringIO(self._exec.stderr) + self.stdin = StringIO(self._exec.stdin) self._container = container - self._exec_id = exec_id + + 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) def wait(self): self._waited = True - self._container.execs[self._exec_id] = dataclasses.replace( - self._container.execs[self._exec_id], - stdin=self.stdin.read(), - ) - exit_code = self._out.return_code + self._store_stdin() + exit_code = self._exec.return_code if exit_code != 0: raise ExecError( list(self._command), @@ -108,12 +113,9 @@ def wait(self): ) def wait_output(self): - out = self._out - self._container.execs[self._exec_id] = dataclasses.replace( - self._container.execs[self._exec_id], - stdin=self.stdin.read(), - ) - exit_code = out.return_code + exec = self._exec + self._store_stdin() + exit_code = exec.return_code if exit_code != 0: raise ExecError( list(self._command), @@ -121,7 +123,7 @@ def wait_output(self): self.stdout.read(), self.stderr.read(), ) - return out.stdout, out.stderr + return exec.stdout, exec.stderr def send_signal(self, sig: Union[int, str]): # noqa: U100 raise NotImplementedError() @@ -759,34 +761,30 @@ def _service_status(self) -> Dict[str, pebble.ServiceStatus]: return self._container.service_status # Based on a method of the same name from ops.testing. - def _find_exec_handler(self, command) -> Tuple[Optional[str], Optional["Exec"]]: - handlers = { - tuple(exec.command): (exec_id, exec) for exec_id, exec in self._container.execs.items() - } + def _find_exec_handler(self, command) -> Optional["Exec"]: + handlers = {exec.command_prefix: exec for exec in self._container.execs} for prefix_len in reversed(range(len(command) + 1)): command_prefix = tuple(command[:prefix_len]) if command_prefix in handlers: return handlers[command_prefix] - return None, None + return None def exec(self, command, **kwargs): # noqa: U100 type: ignore - exec_id, out = self._find_exec_handler(command) - if not out: + handler = self._find_exec_handler(command) + if not handler: raise RuntimeError( f"mock for cmd {command} not found. Please pass to the Container " f"{self._container.name} a scenario.Exec mock for the " f"command your charm is attempting to run, or patch " f"out whatever leads to the call.", ) - assert exec_id is not None # For the static checker. - change_id = out._run() + change_id = handler._run() return _MockExecProcess( change_id=change_id, command=command, - out=out, + exec=handler, container=self._container, - exec_id=exec_id, ) def _check_connection(self): diff --git a/scenario/state.py b/scenario/state.py index 2345ca10..d3e88ff0 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -646,7 +646,7 @@ def _generate_new_change_id(): @dataclasses.dataclass(frozen=True) class Exec(_max_posargs(0)): """Mock data for simulated :meth:`ops.Container.exec` calls.""" - command: Sequence[str] + command_prefix: Sequence[str] return_code: int = 0 """The return code of the process (0 is success).""" stdout: str = "" @@ -828,7 +828,7 @@ class Container(_max_posargs(1)): } """ - execs: Dict[str, Exec] = dataclasses.field(default_factory=dict) + execs: FrozenSet[Exec] = frozenset() """Simulate executing commands in the container. Specify each command the charm might run in the container and a :class:`Exec` @@ -915,6 +915,17 @@ def get_filesystem(self, ctx: "Context") -> Path: """ return ctx._get_container_root(self.name) + def get_exec(self, command_prefix: Sequence[str]): + """Get the Exec object from the container with the given command prefix.""" + for exec in self.execs: + if exec.command_prefix == command_prefix: + 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_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 4474d0ea..5e6f0ce7 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -202,7 +202,7 @@ def callback(self: CharmBase): name="foo", can_connect=True, execs={ - "cmd": Exec([cmd], stdout=out), + Exec((cmd,), stdout=out), }, ) } @@ -315,7 +315,7 @@ def test_exec_wait_error(charm_cls): name="foo", can_connect=True, execs={ - "error": Exec(("foo",), stdout="hello pebble", return_code=1), + Exec(("foo",), stdout="hello pebble", return_code=1), }, ) } @@ -336,9 +336,7 @@ def test_exec_wait_output(charm_cls): Container( name="foo", can_connect=True, - execs={ - "waiter": Exec(("foo",), stdout="hello pebble", stderr="oepsie") - }, + execs={Exec(("foo",), stdout="hello pebble", stderr="oepsie")}, ) } ) @@ -359,7 +357,7 @@ def test_exec_wait_output_error(charm_cls): name="foo", can_connect=True, execs={ - "error": Exec(("foo",), stdout="hello pebble", return_code=1), + Exec(("foo",), stdout="hello pebble", return_code=1), }, ) } From 9077a532ff82f4b83b80653ac23ec4e86fc075fe Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 19:08:26 +1200 Subject: [PATCH 07/40] Mention the exec command prefix matching. --- README.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a7a3cd22..0c44be26 100644 --- a/README.md +++ b/README.md @@ -648,9 +648,9 @@ def test_pebble_exec(): name='foo', execs={ scenario.Exec( - ('ls', '-ll'), # this is the command we're mocking + command_prefix=('ls', '-ll'), return_code=0, - stdout=LS_LL + stdout=LS_LL, ), } ) @@ -666,6 +666,14 @@ def test_pebble_exec(): assert state_out.containers["foo"].get_exec(('ls', '-ll')).stdin = "..." ``` +Scenario will attempt to find the right `Exec` object by matching the provided +command prefix against the command used in the ops `container.exec()` call. For +example, if the command is `['ls', '-ll']` then Scenario will look for an `Exec` +with exactly the same as command prefix, `('ls', '-ll')`, and if not found will +look for an `Exec` with the command prefix `('ls', )`, and if not found will +look for an `Exec` with the command prefix `()`, and if not found will raise +a `RuntimeError`. + ### Pebble Notices Pebble can generate notices, which Juju will detect, and wake up the charm to From d39cfd80144bf8d63f158ea4dc2d490f2101ccf7 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 19:08:39 +1200 Subject: [PATCH 08/40] Undo auto-reformat. --- scenario/mocking.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 4d6faf76..56782328 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -105,12 +105,7 @@ def wait(self): self._store_stdin() exit_code = self._exec.return_code if exit_code != 0: - raise ExecError( - list(self._command), - exit_code, - None, - None, - ) + raise ExecError(list(self._command), exit_code, None, None) def wait_output(self): exec = self._exec From 2d0734a9e9c379e3aa008698c62b1103ff5f57b5 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 19:09:04 +1200 Subject: [PATCH 09/40] Ensure that there aren't two identical command prefixes. --- scenario/consistency_checker.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index e16fb300..15ca3d8e 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -208,6 +208,11 @@ def _check_workload_event( "you **can** fire fire pebble-ready while the container cannot connect, " "but that's most likely not what you want.", ) + command_prefixes = [exec.command_prefix for exec in event.container.execs] + if len(command_prefixes) != len(set(command_prefixes)): + errors.append( + f"container {event.container.name} has multiple execs with the same command prefix.", + ) def _check_action_event( From 514c37f775912c4c2fbd5be311ed64fa94b8742a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 19:28:52 +1200 Subject: [PATCH 10/40] Enable coverage report. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 1652385f..af5693b9 100644 --- a/tox.ini +++ b/tox.ini @@ -28,7 +28,7 @@ deps = setenv = PYTHONPATH = {toxinidir} commands = - pytest --cov-report html:.cov_html -v --tb native --log-cli-level=INFO -s {posargs} {[vars]tst_path} + pytest --cov={[vars]src_path} --cov-report html:.cov_html -v --tb native --log-cli-level=INFO -s {posargs} {[vars]tst_path} [testenv:lint] description = Format the code base to adhere to our styles, and complain about what we cannot do automatically. From dffea4d6b667ff8fa6a2a764b584c7cc6574746a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 19:40:41 +1200 Subject: [PATCH 11/40] Add a test for duplicate exec detection. --- scenario/state.py | 5 +++++ tests/test_consistency_checker.py | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/scenario/state.py b/scenario/state.py index d3e88ff0..18aaf506 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -853,6 +853,11 @@ class Container(_max_posargs(1)): def __hash__(self) -> int: return hash(self.name) + def __post_init__(self): + if not isinstance(self.execs, frozenset): + # Allow passing a regular set (or other iterable) of Execs. + super().__setattr__("execs", frozenset(self.execs)) + def _render_services(self): # copied over from ops.testing._TestingPebbleClient._render_services() services = {} # type: Dict[str, pebble.Service] diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 5695e5b9..7b9388df 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -13,6 +13,8 @@ CloudSpec, Container, Model, + Exec, + Event, Network, Notice, PeerRelation, @@ -23,7 +25,6 @@ Storage, StoredState, SubordinateRelation, - _Action, _CharmSpec, _Event, ) @@ -181,6 +182,15 @@ def test_evt_bad_container_name(): ) +def test_duplicate_execs_in_container(): + container = Container("foo", execs={Exec(("ls", "-l"), return_code=0), Exec(("ls", "-l"), return_code=1)}) + assert_inconsistent( + State(containers=[container]), + Event("foo-pebble-ready", container=container), + _CharmSpec(MyCharm, {"containers": {"foo": {}}}), + ) + + @pytest.mark.parametrize("suffix", RELATION_EVENTS_SUFFIX) def test_evt_bad_relation_name(suffix): assert_inconsistent( From 53f2296966a4058984494cad1c6884dcbbde1754 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 4 Jun 2024 16:10:06 +1200 Subject: [PATCH 12/40] Add addition tests for container.get_exec. --- tests/test_e2e/test_pebble.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 5e6f0ce7..971c2a1a 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -214,6 +214,24 @@ def callback(self: CharmBase): ) +def test_get_exec(): + class MyCharm(CharmBase): + def __init__(self, framework: Framework): + super().__init__(framework) + self.framework.observe(self.on.foo_pebble_ready, self._on_ready) + + def _on_ready(self, _): + proc = self.unit.get_container("foo").exec(["ls"]) + proc.stdin.write("hello world!") + proc.wait_output() + + ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) + exec = Exec((), stdout=LS) + container = Container(name="foo", can_connect=True, execs={exec}) + state_out = ctx.run(container.pebble_ready_event, State(containers=[container])) + assert state_out.get_container(container).get_exec(()).stdin == "hello world!" + + def test_pebble_ready(charm_cls): def callback(self: CharmBase): foo = self.unit.get_container("foo") From f1fe0d1523c092385f97a66cd0976f94105c41a0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 23:15:59 +1200 Subject: [PATCH 13/40] Small adjustments per review. --- README.md | 10 +++++----- scenario/consistency_checker.py | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 0c44be26..98fea922 100644 --- a/README.md +++ b/README.md @@ -668,11 +668,11 @@ def test_pebble_exec(): Scenario will attempt to find the right `Exec` object by matching the provided command prefix against the command used in the ops `container.exec()` call. For -example, if the command is `['ls', '-ll']` then Scenario will look for an `Exec` -with exactly the same as command prefix, `('ls', '-ll')`, and if not found will -look for an `Exec` with the command prefix `('ls', )`, and if not found will -look for an `Exec` with the command prefix `()`, and if not found will raise -a `RuntimeError`. +example if the command is `['ls', '-ll']` then the searching will be: + 1. an `Exec` with exactly the same as command prefix, `('ls', '-ll')` + 2. an `Exec` with the command prefix `('ls', )` + 3. an `Exec` with the command prefix `()` +If none of these are found Scenario will raise a `RuntimeError`. ### Pebble Notices diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index 15ca3d8e..f839bbdc 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -4,7 +4,7 @@ import marshal import os import re -from collections import defaultdict +from collections import Counter, defaultdict from collections.abc import Sequence from numbers import Number from typing import TYPE_CHECKING, Iterable, List, NamedTuple, Tuple, Union @@ -208,10 +208,10 @@ def _check_workload_event( "you **can** fire fire pebble-ready while the container cannot connect, " "but that's most likely not what you want.", ) - command_prefixes = [exec.command_prefix for exec in event.container.execs] - if len(command_prefixes) != len(set(command_prefixes)): + 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 multiple execs with the same command prefix.", + f"container {event.container.name} has duplicate command prefixes: {dupes}", ) From e86d7032ffc39e53e782c367e9ba2a7d46534323 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 13:16:02 +1200 Subject: [PATCH 14/40] Small clean-up. --- README.md | 10 ++++++++++ scenario/mocking.py | 9 ++++++++- scenario/state.py | 2 +- tests/test_consistency_checker.py | 6 ++++-- tests/test_e2e/test_pebble.py | 12 +++--------- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 98fea922..4bf35906 100644 --- a/README.md +++ b/README.md @@ -669,11 +669,21 @@ def test_pebble_exec(): Scenario will attempt to find the right `Exec` object by matching the provided command prefix against the command used in the ops `container.exec()` call. For example if the command is `['ls', '-ll']` then the searching will be: + 1. an `Exec` with exactly the same as command prefix, `('ls', '-ll')` 2. an `Exec` with the command prefix `('ls', )` 3. an `Exec` with the command prefix `()` + If none of these are found Scenario will raise a `RuntimeError`. +Note that the `return_code`, `stdout`, and `stderr` attributes of an `Exec` +object are the *output* that the charm will receive when a matching `exec` call +is made. You'll want to provide these when constructing the input state, and +will rarely want to assert on them in the output state (they cannot change). +The `stdin` attribute is ignored in the input state, and in the output +state will contain any content that the charm wrote to stdin as part of the +`exec` call, so you'll want to assert that it has the appropriate value. + ### Pebble Notices Pebble can generate notices, which Juju will detect, and wake up the charm to diff --git a/scenario/mocking.py b/scenario/mocking.py index 56782328..2e08b054 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -87,7 +87,8 @@ def __init__( self._waited = False self.stdout = StringIO(self._exec.stdout) self.stderr = StringIO(self._exec.stderr) - self.stdin = StringIO(self._exec.stdin) + # You can't pass *in* the stdin, the charm is responsible for that. + self.stdin = StringIO() self._container = container def _store_stdin(self): @@ -758,10 +759,16 @@ def _service_status(self) -> Dict[str, pebble.ServiceStatus]: # Based on a method of the same name from ops.testing. def _find_exec_handler(self, command) -> Optional["Exec"]: handlers = {exec.command_prefix: exec for exec in self._container.execs} + # Start with the full command and, each loop iteration, drop the last + # element, until it matches one of the command prefixes in the execs. + # This includes matching against the empty list, which will match any + # command, if there is not a more specific match. for prefix_len in reversed(range(len(command) + 1)): command_prefix = tuple(command[:prefix_len]) if command_prefix in handlers: return handlers[command_prefix] + # None of the command prefixes in the execs matched the command, no + # matter how much of it was used, so we have failed to find a handler. return None def exec(self, command, **kwargs): # noqa: U100 type: ignore diff --git a/scenario/state.py b/scenario/state.py index 18aaf506..c4d17ed1 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -856,7 +856,7 @@ def __hash__(self) -> int: def __post_init__(self): if not isinstance(self.execs, frozenset): # Allow passing a regular set (or other iterable) of Execs. - super().__setattr__("execs", frozenset(self.execs)) + object.__setattr__(self, "execs", frozenset(self.execs)) def _render_services(self): # copied over from ops.testing._TestingPebbleClient._render_services() diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 7b9388df..1fd82446 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -14,7 +14,6 @@ Container, Model, Exec, - Event, Network, Notice, PeerRelation, @@ -183,7 +182,10 @@ def test_evt_bad_container_name(): def test_duplicate_execs_in_container(): - container = Container("foo", execs={Exec(("ls", "-l"), return_code=0), Exec(("ls", "-l"), return_code=1)}) + container = Container( + "foo", + execs={Exec(("ls", "-l"), return_code=0), Exec(("ls", "-l"), return_code=1)}, + ) assert_inconsistent( State(containers=[container]), Event("foo-pebble-ready", container=container), diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 971c2a1a..532cb7a4 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -201,9 +201,7 @@ def callback(self: CharmBase): Container( name="foo", can_connect=True, - execs={ - Exec((cmd,), stdout=out), - }, + execs={Exec((cmd,), stdout=out)}, ) } ), @@ -332,9 +330,7 @@ def test_exec_wait_error(charm_cls): Container( name="foo", can_connect=True, - execs={ - Exec(("foo",), stdout="hello pebble", return_code=1), - }, + execs={Exec(("foo",), stdout="hello pebble", return_code=1)}, ) } ) @@ -374,9 +370,7 @@ def test_exec_wait_output_error(charm_cls): Container( name="foo", can_connect=True, - execs={ - Exec(("foo",), stdout="hello pebble", return_code=1), - }, + execs={Exec(("foo",), stdout="hello pebble", return_code=1)}, ) } ) From 5e9ab73fa96192cfe2bc94caeeba9b16f1684802 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 7 Jun 2024 17:12:11 +1200 Subject: [PATCH 15/40] Rename State.service_status to State.service_statuses, since there are multiple. --- scenario/mocking.py | 2 +- scenario/state.py | 4 ++-- tests/test_e2e/test_pebble.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 2e08b054..91285075 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -754,7 +754,7 @@ def _layers(self) -> Dict[str, pebble.Layer]: @property def _service_status(self) -> Dict[str, pebble.ServiceStatus]: - return self._container.service_status + return self._container.service_statuses # Based on a method of the same name from ops.testing. def _find_exec_handler(self, command) -> Optional["Exec"]: diff --git a/scenario/state.py b/scenario/state.py index c4d17ed1..77597808 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -803,7 +803,7 @@ class Container(_max_posargs(1)): layers: Dict[str, pebble.Layer] = dataclasses.field(default_factory=dict) """All :class:`ops.pebble.Layer` definitions that have already been added to the container.""" - service_status: Dict[str, pebble.ServiceStatus] = dataclasses.field( + service_statuses: Dict[str, pebble.ServiceStatus] = dataclasses.field( default_factory=dict, ) """The current status of each Pebble service running in the container.""" @@ -898,7 +898,7 @@ def services(self) -> Dict[str, pebble.ServiceInfo]: # in pebble, it just returns "nothing matched" if there are 0 matches, # but it ignores services it doesn't recognize continue - status = self.service_status.get(name, pebble.ServiceStatus.INACTIVE) + status = self.service_statuses.get(name, pebble.ServiceStatus.INACTIVE) if service.startup == "": startup = pebble.ServiceStartup.DISABLED else: diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 532cb7a4..5f221456 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -297,7 +297,7 @@ def _on_ready(self, event): } ) }, - service_status={ + service_statuses={ "fooserv": pebble.ServiceStatus.ACTIVE, # todo: should we disallow setting status for services that aren't known YET? "barserv": starting_service_status, From fb75c869544fff5a2d7eabd6f30da8a2b58fd9a9 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 30 May 2024 19:40:56 +1200 Subject: [PATCH 16/40] Fix broken files. --- scenario/mocking.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 91285075..1aaa4951 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -582,7 +582,7 @@ def action_get(self): def storage_add(self, name: str, count: int = 1): if not isinstance(count, int) or isinstance(count, bool): raise TypeError( - f"storage count must be integer, got: {count} ({type(count)})", + f"storage count must be integer, got: {count} ({type(count)}", ) if "/" in name: From 52f517c7b2377d25a05359179a08e4197e3ad1cb Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 30 May 2024 19:43:14 +1200 Subject: [PATCH 17/40] Update scenario/mocking.py --- scenario/mocking.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 1aaa4951..91285075 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -582,7 +582,7 @@ def action_get(self): def storage_add(self, name: str, count: int = 1): if not isinstance(count, int) or isinstance(count, bool): raise TypeError( - f"storage count must be integer, got: {count} ({type(count)}", + f"storage count must be integer, got: {count} ({type(count)})", ) if "/" in name: From 36327d6e080e4baac3e9ecf34497f4f00da425c0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 4 Jun 2024 16:44:06 +1200 Subject: [PATCH 18/40] Update tests and docs to match final (hopefully\!) API decision. --- tests/test_context_on.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 151bc303..f2805c4c 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -153,7 +153,7 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(event_name, event_kind): +def test_storage_events(as_kwarg, storage_index, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) storage = scenario.Storage("foo") state_in = scenario.State(storages=[storage]) @@ -165,8 +165,17 @@ def test_storage_events(event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storage.name - assert event.storage.index == storage.index + assert event.storage.name == storages[-1].name + assert event.storage.index == storages[-1].index + + +@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) +def test_storage_events_as_positional_arg(event_name): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) + with pytest.assertRaises(ValueError): + ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) def test_action_event_no_params(): From d3f9916df54a9cb6bfd8b1f1f4520b45238c5e36 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 10:51:42 +1200 Subject: [PATCH 19/40] Fix tests. The failing test are (a) ones that need to be rewritten for the new system, (b) ones to do with custom events. --- tests/test_context_on.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index f2805c4c..151bc303 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -153,7 +153,7 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(as_kwarg, storage_index, event_name, event_kind): +def test_storage_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) storage = scenario.Storage("foo") state_in = scenario.State(storages=[storage]) @@ -165,17 +165,8 @@ def test_storage_events(as_kwarg, storage_index, event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storages[-1].name - assert event.storage.index == storages[-1].index - - -@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) -def test_storage_events_as_positional_arg(event_name): - ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storage = scenario.Storage("foo") - state_in = scenario.State(storage=[storage]) - with pytest.assertRaises(ValueError): - ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) + assert event.storage.name == storage.name + assert event.storage.index == storage.index def test_action_event_no_params(): From 38c7a71c6900a8bdeab3ccbbc7ee158268469acc Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 18:38:53 +1200 Subject: [PATCH 20/40] Move the checks that were on binding to the consistency checker. --- tests/test_consistency_checker.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 1fd82446..36a8ab0a 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -360,7 +360,6 @@ def test_secret_not_in_state(): _CharmSpec(MyCharm, {}), ) - def test_peer_relation_consistency(): assert_inconsistent( State(relations={Relation("foo")}), @@ -415,6 +414,19 @@ def test_relation_sub_inconsistent(): _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), ) +def test_relation_not_in_state(): + relation = Relation("foo") + assert_inconsistent( + State(), + _Event("foo_relation_changed", relation=relation), + _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), + ) + assert_consistent( + State(relations=[relation]), + _Event("foo_relation_changed", relation=relation), + _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), + ) + def test_relation_not_in_state(): relation = Relation("foo") From dbfbb91088c205bd92c83dae8ae92c08ddb83ab6 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 19:36:44 +1200 Subject: [PATCH 21/40] Update tests now that emitting custom events is not possible. --- tests/test_consistency_checker.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 36a8ab0a..2cb77f00 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -414,6 +414,7 @@ def test_relation_sub_inconsistent(): _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), ) + def test_relation_not_in_state(): relation = Relation("foo") assert_inconsistent( From 3492ab173df1d7a51505884ef5592859da30ee95 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 24 Apr 2024 14:31:41 +1200 Subject: [PATCH 22/40] Support 'ctx.on.event_name' for specifying events. --- scenario/context.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scenario/context.py b/scenario/context.py index 6dcf910a..20451ccb 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -682,6 +682,11 @@ 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( From 0cad14cf2dcf3922449674ce5873814d7894d56d Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 4 Jun 2024 16:44:06 +1200 Subject: [PATCH 23/40] Update tests and docs to match final (hopefully\!) API decision. --- tests/test_context_on.py | 19 +++++++++++++------ tests/test_e2e/test_event.py | 4 +--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/test_context_on.py b/tests/test_context_on.py index 151bc303..b8909761 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -153,7 +153,7 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(event_name, event_kind): +def test_storage_events(as_kwarg, storage_index, event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) storage = scenario.Storage("foo") state_in = scenario.State(storages=[storage]) @@ -165,8 +165,17 @@ def test_storage_events(event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storage.name - assert event.storage.index == storage.index + assert event.storage.name == storages[-1].name + assert event.storage.index == storages[-1].index + + +@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) +def test_storage_events_as_positional_arg(event_name): + ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) + storage = scenario.Storage("foo") + state_in = scenario.State(storage=[storage]) + with pytest.assertRaises(ValueError): + ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) def test_action_event_no_params(): @@ -308,9 +317,7 @@ def test_relation_unit_events(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) - with ctx.manager( - getattr(ctx.on, event_name)(relation, remote_unit=2), state_in - ) as mgr: + with ctx.manager(getattr(ctx.on, event_name)(relation, unit=2), state_in) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index 409f225b..178eb11b 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -86,9 +86,7 @@ def _foo(self, e): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run( - ctx.on.start(), State(deferred=[_Event("update-status").deferred(MyCharm._foo)]) - ) + ctx.run(ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)])) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ From 3e4c57449a65fcd88266e3f00f417e825a38c887 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 5 Jun 2024 10:51:42 +1200 Subject: [PATCH 24/40] Fix tests. The failing test are (a) ones that need to be rewritten for the new system, (b) ones to do with custom events. --- scenario/state.py | 1 - tests/test_context_on.py | 19 ++++++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/scenario/state.py b/scenario/state.py index 77597808..16752e92 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -1189,7 +1189,6 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated filesystem root in this context.""" return ctx._get_storage_root(self.name, self.index) - @dataclasses.dataclass(frozen=True) class Resource(_max_posargs(0)): """Represents a resource made available to the charm.""" diff --git a/tests/test_context_on.py b/tests/test_context_on.py index b8909761..151bc303 100644 --- a/tests/test_context_on.py +++ b/tests/test_context_on.py @@ -153,7 +153,7 @@ def test_revision_secret_events_as_positional_arg(event_name): ("storage_detaching", ops.StorageDetachingEvent), ], ) -def test_storage_events(as_kwarg, storage_index, event_name, event_kind): +def test_storage_events(event_name, event_kind): ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) storage = scenario.Storage("foo") state_in = scenario.State(storages=[storage]) @@ -165,17 +165,8 @@ def test_storage_events(as_kwarg, storage_index, event_name, event_kind): assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) event = mgr.charm.observed[0] assert isinstance(event, event_kind) - assert event.storage.name == storages[-1].name - assert event.storage.index == storages[-1].index - - -@pytest.mark.parametrize("event_name", ["storage_attached", "storage_detaching"]) -def test_storage_events_as_positional_arg(event_name): - ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS) - storage = scenario.Storage("foo") - state_in = scenario.State(storage=[storage]) - with pytest.assertRaises(ValueError): - ctx.run(getattr(ctx.on, event_name)(storage, 0), state_in) + assert event.storage.name == storage.name + assert event.storage.index == storage.index def test_action_event_no_params(): @@ -317,7 +308,9 @@ def test_relation_unit_events(event_name, event_kind): state_in = scenario.State(relations=[relation]) # These look like: # ctx.run(ctx.on.baz_relation_changed(unit=unit_ordinal), state) - with ctx.manager(getattr(ctx.on, event_name)(relation, unit=2), state_in) as mgr: + with ctx.manager( + getattr(ctx.on, event_name)(relation, remote_unit=2), state_in + ) as mgr: mgr.run() assert len(mgr.charm.observed) == 2 assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent) From 03ef892afefe64fa3e8b52c0fcbf1490b778ba24 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 6 Jun 2024 18:47:14 +1200 Subject: [PATCH 25/40] Style fixes. --- scenario/state.py | 1 + tests/test_e2e/test_event.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/scenario/state.py b/scenario/state.py index 16752e92..77597808 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -1189,6 +1189,7 @@ def get_filesystem(self, ctx: "Context") -> Path: """Simulated filesystem root in this context.""" return ctx._get_storage_root(self.name, self.index) + @dataclasses.dataclass(frozen=True) class Resource(_max_posargs(0)): """Represents a resource made available to the charm.""" diff --git a/tests/test_e2e/test_event.py b/tests/test_e2e/test_event.py index 178eb11b..409f225b 100644 --- a/tests/test_e2e/test_event.py +++ b/tests/test_e2e/test_event.py @@ -86,7 +86,9 @@ def _foo(self, e): capture_deferred_events=True, capture_framework_events=True, ) - ctx.run(ctx.on.start(), State(deferred=[Event("update-status").deferred(MyCharm._foo)])) + ctx.run( + ctx.on.start(), State(deferred=[_Event("update-status").deferred(MyCharm._foo)]) + ) assert len(ctx.emitted_events) == 5 assert [e.handle.kind for e in ctx.emitted_events] == [ From 432f50ad1e2581e77cceedfe29d44e404ff385ef Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 18 Jun 2024 17:30:49 +1200 Subject: [PATCH 26/40] Post-merge fixes. --- scenario/consistency_checker.py | 2 +- tests/test_consistency_checker.py | 2 +- tests/test_e2e/test_pebble.py | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index f839bbdc..d8a233aa 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -186,7 +186,7 @@ def _check_workload_event( event: "_Event", state: "State", errors: List[str], - warnings: List[str], # noqa: U100 + warnings: List[str], ): if not event.container: errors.append( diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 2cb77f00..8b52120b 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -188,7 +188,7 @@ def test_duplicate_execs_in_container(): ) assert_inconsistent( State(containers=[container]), - Event("foo-pebble-ready", container=container), + _Event("foo-pebble-ready", container=container), _CharmSpec(MyCharm, {"containers": {"foo": {}}}), ) diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 5f221456..a7e00e80 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -226,7 +226,9 @@ def _on_ready(self, _): ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) exec = Exec((), stdout=LS) container = Container(name="foo", can_connect=True, execs={exec}) - state_out = ctx.run(container.pebble_ready_event, State(containers=[container])) + state_out = ctx.run( + ctx.on.pebble_ready(container=container), State(containers=[container]) + ) assert state_out.get_container(container).get_exec(()).stdin == "hello world!" From 02d59be041d16531d911427f53091317947268e0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 12:26:18 +1200 Subject: [PATCH 27/40] 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 == {} From 4221b02173372b9e779f09c276d3ccf7fe14086e Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 13:17:43 +1200 Subject: [PATCH 28/40] Simplify code. --- scenario/mocking.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index 6221baf3..a7e50ab4 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -88,19 +88,16 @@ def __init__( # You can't pass *in* the stdin, the charm is responsible for that. self.stdin = StringIO() - def _store_stdin(self): - self._exec._update_stdin(self.stdin.getvalue()) - def wait(self): self._waited = True - self._store_stdin() + self._exec._update_stdin(self.stdin.getvalue()) exit_code = self._exec.return_code if exit_code != 0: raise ExecError(list(self._command), exit_code, None, None) def wait_output(self): exec = self._exec - self._store_stdin() + self._exec._update_stdin(self.stdin.getvalue()) exit_code = exec.return_code if exit_code != 0: raise ExecError( From e3a15bba56ec2324444e44915374b9662adf94b9 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 13:19:32 +1200 Subject: [PATCH 29/40] Grammar fix. --- scenario/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario/state.py b/scenario/state.py index 03365cf4..b21e933e 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -836,7 +836,7 @@ class Container(_max_posargs(1)): execs: Iterable[Exec] = frozenset() """Simulate executing commands in the container. - Specify each command the charm might run in the container and a :class:`Exec` + Specify each command the charm might run in the container and an :class:`Exec` containing its return code and any stdout/stderr. For example:: From cc8b3ca3fcb313f5704ba5bf86093d8a3980b057 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 13:20:42 +1200 Subject: [PATCH 30/40] Fix docstring example. --- scenario/state.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scenario/state.py b/scenario/state.py index b21e933e..d56f14f8 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -844,9 +844,12 @@ class Container(_max_posargs(1)): container = scenario.Container( name='foo', execs={ - ('whoami', ): scenario.Exec(return_code=0, stdout='ubuntu') - ('dig', '+short', 'canonical.com'): - scenario.Exec(return_code=0, stdout='185.125.190.20\\n185.125.190.21') + scenario.Exec(('whoami', ), return_code=0, stdout='ubuntu'), + scenario.Exec( + ('dig', '+short', 'canonical.com'), + return_code=0, + stdout='185.125.190.20\\n185.125.190.21', + ), } ) """ From df5462fdd0884d75e9c15f5c3d9d87a5c0d7f7c9 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 13:23:29 +1200 Subject: [PATCH 31/40] Fix README. --- README.md | 8 ++++---- scenario/consistency_checker.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 4bf35906..24ae42b3 100644 --- a/README.md +++ b/README.md @@ -496,7 +496,7 @@ If you want to, you can override any of these relation or extra-binding associat ```python state = scenario.State(networks={ - scenario.Network("foo", [BindAddress([Address('192.0.2.1')])]) + scenario.Network("foo", [scenario.BindAddress([scenario.Address('192.0.2.1')])]) }) ``` @@ -663,7 +663,7 @@ def test_pebble_exec(): ctx.on.pebble_ready(container), state_in, ) - assert state_out.containers["foo"].get_exec(('ls', '-ll')).stdin = "..." + assert state_out.containers["foo"].get_exec(('ls', '-ll')).stdin == "..." ``` Scenario will attempt to find the right `Exec` object by matching the provided @@ -726,9 +726,9 @@ check doesn't have to match the event being generated: by the time that Juju sends a pebble-check-failed event the check might have started passing again. ```python -ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": {"my-container": {}}}) +ctx = scenario.Context(MyCharm, meta={"name": "foo", "containers": {"my_container": {}}}) check_info = scenario.CheckInfo("http-check", failures=7, status=ops.pebble.CheckStatus.DOWN) -container = scenario.Container("my-container", check_infos={check_info}) +container = scenario.Container("my_container", check_infos={check_info}) state = scenario.State(containers={container}) ctx.run(ctx.on.pebble_check_failed(info=check_info, container=container), state=state) ``` diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index dcb46652..b0c13d23 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -599,7 +599,7 @@ def check_containers_consistency( ): errors.append( f"the event being processed concerns check {event.check_info.name}, but that " - "check is not the {evt_container_name} container.", + f"check is not the {evt_container_name} container.", ) # - a container in state.containers is not in meta.containers From 7fe631da1418133d8a51de0d74694850297990f8 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 13:25:08 +1200 Subject: [PATCH 32/40] Remove duplicated code. --- tests/test_consistency_checker.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 769c84e5..8f7fce2e 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -416,20 +416,6 @@ def test_relation_sub_inconsistent(): ) -def test_relation_not_in_state(): - relation = Relation("foo") - assert_inconsistent( - State(), - _Event("foo_relation_changed", relation=relation), - _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), - ) - assert_consistent( - State(relations=[relation]), - _Event("foo_relation_changed", relation=relation), - _CharmSpec(MyCharm, {"requires": {"foo": {"interface": "bar"}}}), - ) - - def test_relation_not_in_state(): relation = Relation("foo") assert_inconsistent( From ee72367eda6f586638c15dd22e47cbcc155125f8 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 13:27:07 +1200 Subject: [PATCH 33/40] Remove unintended change. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index af5693b9..1652385f 100644 --- a/tox.ini +++ b/tox.ini @@ -28,7 +28,7 @@ deps = setenv = PYTHONPATH = {toxinidir} commands = - pytest --cov={[vars]src_path} --cov-report html:.cov_html -v --tb native --log-cli-level=INFO -s {posargs} {[vars]tst_path} + pytest --cov-report html:.cov_html -v --tb native --log-cli-level=INFO -s {posargs} {[vars]tst_path} [testenv:lint] description = Format the code base to adhere to our styles, and complain about what we cannot do automatically. From a6319277468005dfcdeb99adc433c6bbb7837f6b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 7 Aug 2024 15:32:34 +1200 Subject: [PATCH 34/40] Use lists for Exec command prefix examples. --- README.md | 4 ++-- scenario/state.py | 11 +++++++++-- tests/test_consistency_checker.py | 2 +- tests/test_e2e/test_pebble.py | 10 +++++----- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 24ae42b3..802e599c 100644 --- a/README.md +++ b/README.md @@ -648,7 +648,7 @@ def test_pebble_exec(): name='foo', execs={ scenario.Exec( - command_prefix=('ls', '-ll'), + command_prefix=['ls', '-ll'], return_code=0, stdout=LS_LL, ), @@ -663,7 +663,7 @@ def test_pebble_exec(): ctx.on.pebble_ready(container), state_in, ) - assert state_out.containers["foo"].get_exec(('ls', '-ll')).stdin == "..." + assert state_out.containers["foo"].get_exec(['ls', '-ll']).stdin == "..." ``` Scenario will attempt to find the right `Exec` object by matching the provided diff --git a/scenario/state.py b/scenario/state.py index d56f14f8..71988e82 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -660,6 +660,13 @@ class Exec(_max_posargs(1)): # change ID: used internally to keep track of mocked processes _change_id: int = dataclasses.field(default_factory=_generate_new_change_id) + def __post_init__(self): + # The command prefix can be any sequence type, and a list is tidier to + # write when there's only one string. However, this object needs to be + # hashable, so can't contain a list. We 'freeze' the sequence to a tuple + # to support that. + object.__setattr__(self, "command_prefix", tuple(self.command_prefix)) + def _run(self) -> int: return self._change_id @@ -844,9 +851,9 @@ class Container(_max_posargs(1)): container = scenario.Container( name='foo', execs={ - scenario.Exec(('whoami', ), return_code=0, stdout='ubuntu'), + scenario.Exec(['whoami'], return_code=0, stdout='ubuntu'), scenario.Exec( - ('dig', '+short', 'canonical.com'), + ['dig', '+short', 'canonical.com'], return_code=0, stdout='185.125.190.20\\n185.125.190.21', ), diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index 8f7fce2e..f9658c89 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -184,7 +184,7 @@ def test_evt_bad_container_name(): def test_duplicate_execs_in_container(): container = Container( "foo", - execs={Exec(("ls", "-l"), return_code=0), Exec(("ls", "-l"), return_code=1)}, + execs={Exec(["ls", "-l"], return_code=0), Exec(["ls", "-l"], return_code=1)}, ) assert_inconsistent( State(containers=[container]), diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index fa50e7a7..c9feecd6 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -201,7 +201,7 @@ def callback(self: CharmBase): Container( name="foo", can_connect=True, - execs={Exec((cmd,), stdout=out)}, + execs={Exec([cmd], stdout=out)}, ) } ), @@ -224,7 +224,7 @@ def _on_ready(self, _): proc.wait_output() ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) - exec = Exec((), stdout=LS) + 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}) @@ -332,7 +332,7 @@ def test_exec_wait_error(charm_cls): Container( name="foo", can_connect=True, - execs={Exec(("foo",), stdout="hello pebble", return_code=1)}, + execs={Exec(["foo"], stdout="hello pebble", return_code=1)}, ) } ) @@ -352,7 +352,7 @@ def test_exec_wait_output(charm_cls): Container( name="foo", can_connect=True, - execs={Exec(("foo",), stdout="hello pebble", stderr="oepsie")}, + execs={Exec(["foo"], stdout="hello pebble", stderr="oepsie")}, ) } ) @@ -372,7 +372,7 @@ def test_exec_wait_output_error(charm_cls): Container( name="foo", can_connect=True, - execs={Exec(("foo",), stdout="hello pebble", return_code=1)}, + execs={Exec(["foo"], stdout="hello pebble", return_code=1)}, ) } ) From 23241cb818527d4c237ad38141bb7438cf800a86 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 8 Aug 2024 17:26:15 +1200 Subject: [PATCH 35/40] Small tweaks based on review. --- scenario/consistency_checker.py | 4 +++- scenario/mocking.py | 14 +++++++++----- scenario/state.py | 31 +++++++++++++++++++++++++++---- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index b0c13d23..7dbbba1c 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -588,18 +588,20 @@ def check_containers_consistency( f"container with that name is not present in the state. It's odd, but " f"consistent, if it cannot connect; but it should at least be there.", ) + # - you're processing a Notice event and that notice is not in any of the containers if event.notice and event.notice.id not in all_notices: errors.append( f"the event being processed concerns notice {event.notice!r}, but that " "notice is not in any of the containers present in the state.", ) + # - you're processing a Check event and that check is not in the check's container if ( event.check_info and (evt_container_name, event.check_info.name) not in all_checks ): errors.append( f"the event being processed concerns check {event.check_info.name}, but that " - f"check is not the {evt_container_name} container.", + f"check is not in the {evt_container_name} container.", ) # - a container in state.containers is not in meta.containers diff --git a/scenario/mocking.py b/scenario/mocking.py index a7e50ab4..51aa1cc4 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -761,11 +761,15 @@ def _find_exec_handler(self, command) -> Optional["Exec"]: def exec(self, command, **kwargs): # noqa: U100 type: ignore handler = self._find_exec_handler(command) if not handler: - raise RuntimeError( - f"mock for cmd {command} not found. Please pass to the Container " - f"{self._container.name} a scenario.Exec mock for the " - f"command your charm is attempting to run, or patch " - f"out whatever leads to the call.", + raise ExecError( + command, + 127, + "", + f"mock for cmd {command} not found. Please patch out whatever " + f"leads to the call, or pass to the Container {self._container.name} " + f"a scenario.Exec mock for the command your charm is attempting " + f"to run, such as " + f"'Container(..., execs={{scenario.Exec({list(command)}, ...)}})'", ) change_id = handler._run() diff --git a/scenario/state.py b/scenario/state.py index 71988e82..1af98df5 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -649,13 +649,28 @@ class Exec(_max_posargs(1)): command_prefix: Sequence[str] return_code: int = 0 - """The return code of the process (0 is success).""" + """The return code of the process. + + Use 0 to mock the process ending successfully, and other values for failure. + """ stdout: str = "" - """Any content written to stdout by the process.""" + """Any content written to stdout by the process. + + Provide content that the real process would write to stdout, which can be + read by the charm. + """ stderr: str = "" - """Any content written to stderr by the process.""" + """Any content written to stderr by the process. + + Provide content that the real process would write to stderr, which can be + read by the charm. + """ stdin: Optional[str] = None - """Any content written to stdin by the charm.""" + """stdin content the charm wrote to the process. + + Cannot be provided in the input state - the output state will contain the + content the charm wrote to the process, to use in assertions. + """ # change ID: used internally to keep track of mocked processes _change_id: int = dataclasses.field(default_factory=_generate_new_change_id) @@ -667,6 +682,14 @@ def __post_init__(self): # to support that. object.__setattr__(self, "command_prefix", tuple(self.command_prefix)) + # The process is being mocked - it can't start with existing stdin + # content; the charm is able to write that. + if self.stdin: + raise ValueError( + "processes cannot start with existing stdin content - write to " + "the process in your charm", + ) + def _run(self) -> int: return self._change_id From 42f21b4aa73d8fe1e9acfcdf7ca6c24307cb5457 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 8 Aug 2024 18:20:40 +1200 Subject: [PATCH 36/40] Fix frozen bypass. --- scenario/state.py | 11 ++++++++--- tests/test_e2e/test_pebble.py | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/scenario/state.py b/scenario/state.py index 1af98df5..7523f3d4 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -665,7 +665,7 @@ class Exec(_max_posargs(1)): Provide content that the real process would write to stderr, which can be read by the charm. """ - stdin: Optional[str] = None + _stdin: Optional[str] = None """stdin content the charm wrote to the process. Cannot be provided in the input state - the output state will contain the @@ -684,7 +684,7 @@ def __post_init__(self): # The process is being mocked - it can't start with existing stdin # content; the charm is able to write that. - if self.stdin: + if self._stdin: raise ValueError( "processes cannot start with existing stdin content - write to " "the process in your charm", @@ -695,7 +695,12 @@ def _run(self) -> int: def _update_stdin(self, stdin: str): # bypass frozen dataclass - object.__setattr__(self, "stdin", stdin) + object.__setattr__(self, "_stdin", stdin) + + @property + def stdin(self): + """The content the charm wrote to the mock process's stdin.""" + return self._stdin @dataclasses.dataclass(frozen=True) diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index c9feecd6..00c59124 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -229,7 +229,7 @@ def _on_ready(self, _): state_out = ctx.run( ctx.on.pebble_ready(container=container), State(containers={container}) ) - assert state_out.get_container(container.name).get_exec(()).stdin == "hello world!" + assert state_out.get_container(container.name).get_exec(())._stdin == "hello world!" def test_pebble_ready(charm_cls): From 15ea896b8199f9e757b28a799ce0b10e5e0ffa2f Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 8 Aug 2024 20:27:54 +1200 Subject: [PATCH 37/40] Fix stdin usage. --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 802e599c..2e700813 100644 --- a/README.md +++ b/README.md @@ -639,7 +639,8 @@ class MyCharm(ops.CharmBase): def _on_start(self, _): foo = self.unit.get_container('foo') proc = foo.exec(['ls', '-ll']) - stdout, _ = proc.wait_output("...") + proc.stdin.write("...") + stdout, _ = proc.wait_output() assert stdout == LS_LL From 8928a0fa56195f7f12980630b4cb04b4dcecb221 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 12 Aug 2024 14:52:38 +1200 Subject: [PATCH 38/40] Move the exec args to a context attribute. --- README.md | 15 ++-- scenario/context.py | 2 + scenario/mocking.py | 126 ++++++++++++++++++++++++++-------- scenario/state.py | 30 -------- tests/test_e2e/test_pebble.py | 33 +++++---- 5 files changed, 123 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index 2e700813..4cfb28de 100644 --- a/README.md +++ b/README.md @@ -649,7 +649,7 @@ def test_pebble_exec(): name='foo', execs={ scenario.Exec( - command_prefix=['ls', '-ll'], + command_prefix=['ls'], return_code=0, stdout=LS_LL, ), @@ -664,7 +664,8 @@ def test_pebble_exec(): ctx.on.pebble_ready(container), state_in, ) - assert state_out.containers["foo"].get_exec(['ls', '-ll']).stdin == "..." + assert ctx.exec_history[container.name].command == ['ls', '-ll'] + assert ctx.exec_history[container.name].stdin == "..." ``` Scenario will attempt to find the right `Exec` object by matching the provided @@ -675,15 +676,7 @@ example if the command is `['ls', '-ll']` then the searching will be: 2. an `Exec` with the command prefix `('ls', )` 3. an `Exec` with the command prefix `()` -If none of these are found Scenario will raise a `RuntimeError`. - -Note that the `return_code`, `stdout`, and `stderr` attributes of an `Exec` -object are the *output* that the charm will receive when a matching `exec` call -is made. You'll want to provide these when constructing the input state, and -will rarely want to assert on them in the output state (they cannot change). -The `stdin` attribute is ignored in the input state, and in the output -state will contain any content that the charm wrote to stdin as part of the -`exec` call, so you'll want to assert that it has the appropriate value. +If none of these are found Scenario will raise an `ExecError`. ### Pebble Notices diff --git a/scenario/context.py b/scenario/context.py index 6dcf910a..639ce72f 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +import collections import dataclasses import tempfile from contextlib import contextmanager @@ -550,6 +551,7 @@ def __init__( self.juju_log: List["JujuLogLine"] = [] self.app_status_history: List["_EntityStatus"] = [] self.unit_status_history: List["_EntityStatus"] = [] + self.exec_history = collections.defaultdict(list) self.workload_version_history: List[str] = [] self.emitted_events: List[EventBase] = [] self.requested_storages: Dict[str, int] = {} diff --git a/scenario/mocking.py b/scenario/mocking.py index 51aa1cc4..9785ce6b 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -4,7 +4,6 @@ import datetime import random import shutil -from io import StringIO from pathlib import Path from typing import ( TYPE_CHECKING, @@ -15,6 +14,7 @@ Mapping, Optional, Set, + TextIO, Tuple, Union, cast, @@ -34,7 +34,7 @@ _ModelBackend, ) from ops.pebble import Client, ExecError -from ops.testing import _TestingPebbleClient +from ops.testing import ExecArgs, _TestingPebbleClient from scenario.logger import logger as scenario_logger from scenario.state import ( @@ -75,38 +75,44 @@ class ActionMissingFromContextError(Exception): class _MockExecProcess: def __init__( self, - command: Tuple[str, ...], change_id: int, - exec: "Exec", + args: ExecArgs, + return_code: int, + stdin: Optional[TextIO], + stdout: Optional[TextIO], + stderr: Optional[TextIO], ): - self._command = command self._change_id = change_id - self._exec = exec + self._args = args + self._return_code = return_code self._waited = False - self.stdout = StringIO(self._exec.stdout) - self.stderr = StringIO(self._exec.stderr) - # You can't pass *in* the stdin, the charm is responsible for that. - self.stdin = StringIO() + self.stdin = stdin + self.stdout = stdout + self.stderr = stderr + + def __del__(self): + if not self._waited: + self._close_stdin() + + def _close_stdin(self): + if self._args.stdin is None and self.stdin is not None: + self.stdin.seek(0) + self._args.stdin = self.stdin.read() def wait(self): + self._close_stdin() self._waited = True - self._exec._update_stdin(self.stdin.getvalue()) - exit_code = self._exec.return_code - if exit_code != 0: - raise ExecError(list(self._command), exit_code, None, None) + if self._return_code != 0: + raise ExecError(list(self._args.command), self._return_code, None, None) def wait_output(self): - exec = self._exec - self._exec._update_stdin(self.stdin.getvalue()) - exit_code = exec.return_code - if exit_code != 0: - raise ExecError( - list(self._command), - exit_code, - self.stdout.read(), - self.stderr.read(), - ) - return exec.stdout, exec.stderr + self._close_stdin() + self._waited = True + stdout = self.stdout.read() if self.stdout is not None else None + stderr = self.stderr.read() if self.stderr is not None else None + if self._return_code != 0: + raise ExecError(list(self._args.command), self._return_code, stdout, stderr) + return stdout, stderr def send_signal(self, sig: Union[int, str]): # noqa: U100 raise NotImplementedError() @@ -180,6 +186,8 @@ def get_pebble(self, socket_path: str) -> "Client": state=self._state, event=self._event, charm_spec=self._charm_spec, + context=self._context, + container_name=container_name, ) def _get_relation_by_id( @@ -684,11 +692,15 @@ def __init__( state: "State", event: "_Event", charm_spec: "_CharmSpec", + context: "Context", + container_name: str, ): self._state = state self.socket_path = socket_path self._event = event self._charm_spec = charm_spec + self._context = context + self._container_name = container_name # wipe just in case if container_root.exists(): @@ -758,7 +770,24 @@ def _find_exec_handler(self, command) -> Optional["Exec"]: # matter how much of it was used, so we have failed to find a handler. return None - def exec(self, command, **kwargs): # noqa: U100 type: ignore + def exec( + self, + command: List[str], + *, + environment: Optional[Dict[str, str]] = None, + working_dir: Optional[str] = None, + timeout: Optional[float] = None, + user_id: Optional[int] = None, + user: Optional[str] = None, + group_id: Optional[int] = None, + group: Optional[str] = None, + stdin: Optional[Union[str, bytes, TextIO]] = None, + stdout: Optional[TextIO] = None, + stderr: Optional[TextIO] = None, + encoding: Optional[str] = "utf-8", + combine_stderr: bool = False, + **kwargs, + ): handler = self._find_exec_handler(command) if not handler: raise ExecError( @@ -772,11 +801,48 @@ def exec(self, command, **kwargs): # noqa: U100 type: ignore f"'Container(..., execs={{scenario.Exec({list(command)}, ...)}})'", ) + if stdin is None: + proc_stdin = self._transform_exec_handler_output("", encoding) + else: + proc_stdin = None + stdin = stdin.read() if hasattr(stdin, "read") else stdin # type: ignore + if stdout is None: + proc_stdout = self._transform_exec_handler_output(handler.stdout, encoding) + else: + proc_stdout = None + stdout.write(handler.stdout) + if stderr is None: + proc_stderr = self._transform_exec_handler_output(handler.stderr, encoding) + else: + proc_stderr = None + stderr.write(handler.stderr) + + args = ExecArgs( + command, + environment or {}, + working_dir, + timeout, + user_id, + user, + group_id, + group, + stdin, # type:ignore # If None, will be replaced by proc_stdin.read() later. + encoding, + combine_stderr, + ) + self._context.exec_history[self._container_name].append(args) + change_id = handler._run() - return _MockExecProcess( - change_id=change_id, - command=command, - exec=handler, + return cast( + pebble.ExecProcess[Any], + _MockExecProcess( + change_id=change_id, + args=args, + return_code=handler.return_code, + stdin=proc_stdin, + stdout=proc_stdout, + stderr=proc_stderr, + ), ) def _check_connection(self): diff --git a/scenario/state.py b/scenario/state.py index 7523f3d4..61f07a74 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -665,12 +665,6 @@ class Exec(_max_posargs(1)): Provide content that the real process would write to stderr, which can be read by the charm. """ - _stdin: Optional[str] = None - """stdin content the charm wrote to the process. - - Cannot be provided in the input state - the output state will contain the - content the charm wrote to the process, to use in assertions. - """ # change ID: used internally to keep track of mocked processes _change_id: int = dataclasses.field(default_factory=_generate_new_change_id) @@ -682,26 +676,9 @@ def __post_init__(self): # to support that. object.__setattr__(self, "command_prefix", tuple(self.command_prefix)) - # The process is being mocked - it can't start with existing stdin - # content; the charm is able to write that. - if self._stdin: - raise ValueError( - "processes cannot start with existing stdin content - write to " - "the process in your charm", - ) - def _run(self) -> int: return self._change_id - def _update_stdin(self, stdin: str): - # bypass frozen dataclass - object.__setattr__(self, "_stdin", stdin) - - @property - def stdin(self): - """The content the charm wrote to the mock process's stdin.""" - return self._stdin - @dataclasses.dataclass(frozen=True) class Mount(_max_posargs(0)): @@ -963,13 +940,6 @@ def get_filesystem(self, ctx: "Context") -> Path: """ return ctx._get_container_root(self.name) - def get_exec(self, command_prefix: Sequence[str]): - """Get the Exec object from the container with the given command prefix.""" - for exec in self.execs: - if exec.command_prefix == command_prefix: - return exec - raise KeyError(f"no exec found with command prefix {command_prefix}") - _RawStatusLiteral = Literal[ "waiting", diff --git a/tests/test_e2e/test_pebble.py b/tests/test_e2e/test_pebble.py index 00c59124..a7da27d9 100644 --- a/tests/test_e2e/test_pebble.py +++ b/tests/test_e2e/test_pebble.py @@ -1,5 +1,6 @@ import dataclasses import datetime +import io import tempfile from pathlib import Path @@ -212,24 +213,30 @@ def callback(self: CharmBase): ) -def test_get_exec(): +@pytest.mark.parametrize( + "stdin,write", + ( + [None, "hello world!"], + ["hello world!", None], + [io.StringIO("hello world!"), None], + ), +) +def test_exec_history_stdin(stdin, write): class MyCharm(CharmBase): def __init__(self, framework: Framework): super().__init__(framework) self.framework.observe(self.on.foo_pebble_ready, self._on_ready) def _on_ready(self, _): - proc = self.unit.get_container("foo").exec(["ls"]) - proc.stdin.write("hello world!") - proc.wait_output() + proc = self.unit.get_container("foo").exec(["ls"], stdin=stdin) + if write: + proc.stdin.write(write) + proc.wait() ctx = Context(MyCharm, meta={"name": "foo", "containers": {"foo": {}}}) - 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}) - ) - assert state_out.get_container(container.name).get_exec(())._stdin == "hello world!" + container = Container(name="foo", can_connect=True, execs={Exec([])}) + ctx.run(ctx.on.pebble_ready(container=container), State(containers={container})) + assert ctx.exec_history[container.name][0].stdin == "hello world!" def test_pebble_ready(charm_cls): @@ -346,7 +353,8 @@ def test_exec_wait_error(charm_cls): assert exc_info.value.stdout == "hello pebble" -def test_exec_wait_output(charm_cls): +@pytest.mark.parametrize("command", (["foo"], ["foo", "bar"], ["foo", "bar", "baz"])) +def test_exec_wait_output(charm_cls, command): state = State( containers={ Container( @@ -360,10 +368,11 @@ def test_exec_wait_output(charm_cls): ctx = Context(charm_cls, meta={"name": "foo", "containers": {"foo": {}}}) with ctx.manager(ctx.on.start(), state) as mgr: container = mgr.charm.unit.get_container("foo") - proc = container.exec(["foo"]) + proc = container.exec(command) out, err = proc.wait_output() assert out == "hello pebble" assert err == "oepsie" + assert ctx.exec_history[container.name][0].command == command def test_exec_wait_output_error(charm_cls): From 71525f01c008d09d4434af35ec2065a82b72c35d Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 12 Aug 2024 15:03:32 +1200 Subject: [PATCH 39/40] Fix merge. --- scenario/context.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scenario/context.py b/scenario/context.py index 19ac2f67..760bacf0 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -2,7 +2,6 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. import collections -import dataclasses import tempfile from contextlib import contextmanager from pathlib import Path From 500083609db1c42dce0613caba8be92fdf4e3833 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 12 Aug 2024 16:50:23 +1200 Subject: [PATCH 40/40] Minor fixes based on review. --- README.md | 4 ++-- scenario/context.py | 4 ++-- scenario/mocking.py | 27 +++++++++++++++------------ 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index a088325e..3d313b7d 100644 --- a/README.md +++ b/README.md @@ -664,8 +664,8 @@ def test_pebble_exec(): ctx.on.pebble_ready(container), state_in, ) - assert ctx.exec_history[container.name].command == ['ls', '-ll'] - assert ctx.exec_history[container.name].stdin == "..." + assert ctx.exec_history[container.name][0].command == ['ls', '-ll'] + assert ctx.exec_history[container.name][0].stdin == "..." ``` Scenario will attempt to find the right `Exec` object by matching the provided diff --git a/scenario/context.py b/scenario/context.py index 760bacf0..0f7ca1e1 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -1,13 +1,13 @@ #!/usr/bin/env python3 # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -import collections import tempfile from contextlib import contextmanager from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type, Union, cast from ops import CharmBase, EventBase +from ops.testing import ExecArgs from scenario.logger import logger as scenario_logger from scenario.runtime import Runtime @@ -451,7 +451,7 @@ def __init__( self.juju_log: List["JujuLogLine"] = [] self.app_status_history: List["_EntityStatus"] = [] self.unit_status_history: List["_EntityStatus"] = [] - self.exec_history = collections.defaultdict(list) + self.exec_history: Dict[str, List[ExecArgs]] = {} self.workload_version_history: List[str] = [] self.removed_secret_revisions: List[int] = [] self.emitted_events: List[EventBase] = [] diff --git a/scenario/mocking.py b/scenario/mocking.py index 508af7fc..f906d083 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -844,19 +844,22 @@ def exec( stderr.write(handler.stderr) args = ExecArgs( - command, - environment or {}, - working_dir, - timeout, - user_id, - user, - group_id, - group, - stdin, # type:ignore # If None, will be replaced by proc_stdin.read() later. - encoding, - combine_stderr, + command=command, + environment=environment or {}, + working_dir=working_dir, + timeout=timeout, + user_id=user_id, + user=user, + group_id=group_id, + group=group, + stdin=stdin, # type:ignore # If None, will be replaced by proc_stdin.read() later. + encoding=encoding, + combine_stderr=combine_stderr, ) - self._context.exec_history[self._container_name].append(args) + try: + self._context.exec_history[self._container_name].append(args) + except KeyError: + self._context.exec_history[self._container_name] = [args] change_id = handler._run() return cast(