Skip to content

Commit

Permalink
Post-merge fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyandrewmeyer committed Aug 7, 2024
1 parent 432f50a commit 02d59be
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 76 deletions.
66 changes: 18 additions & 48 deletions scenario/consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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, [])
5 changes: 0 additions & 5 deletions scenario/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 1 addition & 12 deletions scenario/mocking.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -79,7 +78,6 @@ def __init__(
command: Tuple[str, ...],
change_id: int,
exec: "Exec",
container: "ContainerSpec",
):
self._command = command
self._change_id = change_id
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
13 changes: 7 additions & 6 deletions scenario/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)."""
Expand All @@ -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)):
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion tests/test_consistency_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
CloudCredential,
CloudSpec,
Container,
Model,
Exec,
Model,
Network,
Notice,
PeerRelation,
Expand Down Expand Up @@ -360,6 +360,7 @@ def test_secret_not_in_state():
_CharmSpec(MyCharm, {}),
)


def test_peer_relation_consistency():
assert_inconsistent(
State(relations={Relation("foo")}),
Expand Down
4 changes: 2 additions & 2 deletions tests/test_e2e/test_pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_e2e/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == {}

Expand Down

0 comments on commit 02d59be

Please sign in to comment.