From 505f65f22109a06a622d0fbb0429e06c3b7352a7 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 10 Sep 2024 22:16:33 +1200 Subject: [PATCH 1/4] Remove dependency on ops.testing. --- pyproject.toml | 4 +-- scenario/context.py | 21 ++++++++++---- scenario/mocking.py | 46 ++++++++++++++++++++----------- scenario/runtime.py | 4 +-- scenario/state.py | 2 +- tests/helpers.py | 4 +-- tests/test_charm_spec_autoload.py | 5 +--- tox.ini | 2 +- 8 files changed, 53 insertions(+), 35 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f8c89a48..9de5d6d3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "ops-scenario" -version = "7.0.1" +version = "7.0.2" authors = [ { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" } @@ -18,7 +18,7 @@ license.text = "Apache-2.0" keywords = ["juju", "test"] dependencies = [ - "ops~=2.15", + "ops~=2.17", "PyYAML>=6.0.1", ] readme = "README.md" diff --git a/scenario/context.py b/scenario/context.py index cb5331d5..4f4e409b 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -9,7 +9,6 @@ from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Type, Union, cast import ops -import ops.testing from scenario.errors import AlreadyEmittedError, ContextSetupError from scenario.logger import logger as scenario_logger @@ -28,8 +27,17 @@ ) if TYPE_CHECKING: # pragma: no cover + from ops._private.harness import ExecArgs + from scenario.ops_main_mock import Ops - from scenario.state import AnyJson, JujuLogLine, RelationBase, State, _EntityStatus + from scenario.state import ( + AnyJson, + CharmType, + JujuLogLine, + RelationBase, + State, + _EntityStatus, + ) logger = scenario_logger.getChild("runtime") @@ -426,7 +434,7 @@ def test_foo(): def __init__( self, - charm_type: Type[ops.testing.CharmType], + charm_type: Type["CharmType"], meta: Optional[Dict[str, Any]] = None, *, actions: Optional[Dict[str, Any]] = None, @@ -508,7 +516,7 @@ def __init__( self.juju_log: List["JujuLogLine"] = [] self.app_status_history: List["_EntityStatus"] = [] self.unit_status_history: List["_EntityStatus"] = [] - self.exec_history: Dict[str, List[ops.testing.ExecArgs]] = {} + self.exec_history: Dict[str, List["ExecArgs"]] = {} self.workload_version_history: List[str] = [] self.removed_secret_revisions: List[int] = [] self.emitted_events: List[ops.EventBase] = [] @@ -644,7 +652,10 @@ def run(self, event: "_Event", state: "State") -> "State": assert self._output_state is not None if event.action: if self._action_failure_message is not None: - raise ActionFailed(self._action_failure_message, self._output_state) + raise ActionFailed( + self._action_failure_message, + state=self._output_state, + ) return self._output_state @contextmanager diff --git a/scenario/mocking.py b/scenario/mocking.py index f5207a37..b2542e8c 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -1,7 +1,9 @@ #!/usr/bin/env python3 # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. + import datetime +import io import shutil from pathlib import Path from typing import ( @@ -20,6 +22,7 @@ ) from ops import JujuVersion, pebble +from ops._private.harness import ExecArgs, _TestingPebbleClient from ops.model import CloudSpec as CloudSpec_Ops from ops.model import ModelError from ops.model import Port as Port_Ops @@ -33,7 +36,6 @@ _ModelBackend, ) from ops.pebble import Client, ExecError -from ops.testing import ExecArgs, _TestingPebbleClient from scenario.errors import ActionMissingFromContextError from scenario.logger import logger as scenario_logger @@ -66,9 +68,9 @@ def __init__( change_id: int, args: ExecArgs, return_code: int, - stdin: Optional[TextIO], - stdout: Optional[TextIO], - stderr: Optional[TextIO], + stdin: Optional[Union[TextIO, io.BytesIO]], + stdout: Optional[Union[TextIO, io.BytesIO]], + stderr: Optional[Union[TextIO, io.BytesIO]], ): self._change_id = change_id self._args = args @@ -99,7 +101,12 @@ def wait_output(self): 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) + raise ExecError( + list(self._args.command), + self._return_code, + stdout, # type: ignore + stderr, # type: ignore + ) return stdout, stderr def send_signal(self, sig: Union[int, str]): # noqa: U100 @@ -167,15 +174,18 @@ def get_pebble(self, socket_path: str) -> "Client": # container not defined in state. mounts = {} - return _MockPebbleClient( - socket_path=socket_path, - container_root=container_root, - mounts=mounts, - state=self._state, - event=self._event, - charm_spec=self._charm_spec, - context=self._context, - container_name=container_name, + return cast( + Client, + _MockPebbleClient( + socket_path=socket_path, + container_root=container_root, + mounts=mounts, + state=self._state, + event=self._event, + charm_spec=self._charm_spec, + context=self._context, + container_name=container_name, + ), ) def _get_relation_by_id(self, rel_id) -> "RelationBase": @@ -607,7 +617,7 @@ def storage_add(self, name: str, count: int = 1): ) if "/" in name: - # this error is raised by ops.testing but not by ops at runtime + # this error is raised by Harness but not by ops at runtime raise ModelError('storage name cannot contain "/"') self._context.requested_storages[name] = count @@ -743,6 +753,10 @@ def __init__( self._root = container_root + self._notices: Dict[Tuple[str, str], pebble.Notice] = {} + self._last_notice_id = 0 + self._changes: Dict[str, pebble.Change] = {} + # load any existing notices and check information from the state self._notices: Dict[Tuple[str, str], pebble.Notice] = {} self._check_infos: Dict[str, pebble.CheckInfo] = {} @@ -781,7 +795,7 @@ def _layers(self) -> Dict[str, pebble.Layer]: def _service_status(self) -> Dict[str, pebble.ServiceStatus]: return self._container.service_statuses - # Based on a method of the same name from ops.testing. + # Based on a method of the same name from Harness. 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 diff --git a/scenario/runtime.py b/scenario/runtime.py index f4df73db..92cfcae7 100644 --- a/scenario/runtime.py +++ b/scenario/runtime.py @@ -38,10 +38,8 @@ ) if TYPE_CHECKING: # pragma: no cover - from ops.testing import CharmType - from scenario.context import Context - from scenario.state import State, _CharmSpec, _Event + from scenario.state import CharmType, State, _CharmSpec, _Event logger = scenario_logger.getChild("runtime") STORED_STATE_REGEX = re.compile( diff --git a/scenario/state.py b/scenario/state.py index 9179735d..b089c582 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -117,7 +117,7 @@ class ActionFailed(Exception): """Raised at the end of the hook if the charm has called ``event.fail()``.""" - def __init__(self, message: str, state: "State"): + def __init__(self, message: str, *, state: "State"): self.message = message self.state = state diff --git a/tests/helpers.py b/tests/helpers.py index 5ceffa9d..49ffaeff 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -18,9 +18,7 @@ from scenario.context import _DEFAULT_JUJU_VERSION, Context if TYPE_CHECKING: # pragma: no cover - from ops.testing import CharmType - - from scenario.state import State, _Event + from scenario.state import CharmType, State, _Event _CT = TypeVar("_CT", bound=Type[CharmType]) diff --git a/tests/test_charm_spec_autoload.py b/tests/test_charm_spec_autoload.py index 57b93a31..b9da4d24 100644 --- a/tests/test_charm_spec_autoload.py +++ b/tests/test_charm_spec_autoload.py @@ -1,18 +1,15 @@ import importlib import sys -import tempfile from contextlib import contextmanager from pathlib import Path from typing import Type import pytest import yaml -from ops import CharmBase -from ops.testing import CharmType from scenario import Context, Relation, State from scenario.context import ContextSetupError -from scenario.state import MetadataNotFoundError, _CharmSpec +from scenario.state import CharmType, MetadataNotFoundError, _CharmSpec CHARM = """ from ops import CharmBase diff --git a/tox.ini b/tox.ini index d3155731..bfe802f1 100644 --- a/tox.ini +++ b/tox.ini @@ -43,7 +43,7 @@ commands = description = Static typing checks. skip_install = true deps = - ops~=2.15 + ops~=2.17 pyright==1.1.347 commands = pyright scenario From 842e4696bf6f10a4420f645f4f2e7d0dc0e0ee6b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 12 Sep 2024 14:54:56 +1200 Subject: [PATCH 2/4] Assume that the adjustment for Secret._canonicalize_id will come before this. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9de5d6d3..253924d0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "ops-scenario" -version = "7.0.2" +version = "7.0.3" authors = [ { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" } From 16137ca19a174b6e0fc4830b251e4d87ca034bef Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 12 Sep 2024 14:35:35 +1200 Subject: [PATCH 3/4] Handle the changes coming in ops 2.17 --- scenario/mocking.py | 19 ++++++++++++++----- scenario/ops_main_mock.py | 13 +++++++++---- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/scenario/mocking.py b/scenario/mocking.py index b2542e8c..99b5d768 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -209,11 +209,20 @@ def _get_secret(self, id=None, label=None): # in scenario, you can create Secret(id="foo"), # but ops.Secret will prepend a "secret:" prefix to that ID. # we allow getting secret by either version. - secrets = [ - s - for s in self._state.secrets - if canonicalize_id(s.id) == canonicalize_id(id) - ] + try: + secrets = [ + s + for s in self._state.secrets + if canonicalize_id(s.id, model_uuid=self._state.model.uuid) # type: ignore + == canonicalize_id(id, model_uuid=self._state.model.uuid) # type: ignore + ] + except TypeError: + # ops 2.16 + secrets = [ + s + for s in self._state.secrets + if canonicalize_id(s.id) == canonicalize_id(id) # type: ignore + ] if not secrets: raise SecretNotFoundError(id) return secrets[0] diff --git a/scenario/ops_main_mock.py b/scenario/ops_main_mock.py index 27f00a8b..786b113f 100644 --- a/scenario/ops_main_mock.py +++ b/scenario/ops_main_mock.py @@ -12,13 +12,18 @@ import ops.model import ops.storage from ops import CharmBase + +# use logger from ops._main so that juju_log will be triggered +try: + from ops._main import CHARM_STATE_FILE, _Dispatcher, _get_event_args # type: ignore + from ops._main import logger as ops_logger # type: ignore +except ImportError: + # Ops 2.16 + from ops.main import CHARM_STATE_FILE, _Dispatcher, _get_event_args # type: ignore + from ops.main import logger as ops_logger # type: ignore from ops.charm import CharmMeta from ops.log import setup_root_logging -# use logger from ops.main so that juju_log will be triggered -from ops.main import CHARM_STATE_FILE, _Dispatcher, _get_event_args -from ops.main import logger as ops_logger - from scenario.errors import BadOwnerPath, NoObserverError if TYPE_CHECKING: # pragma: no cover From 6686bb588cbf29d8bac26818672e5f175460f71b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 13 Sep 2024 12:08:43 +1200 Subject: [PATCH 4/4] Handle both ops 2.15+ and ops 2.17 --- pyproject.toml | 2 +- scenario/context.py | 7 +++++-- scenario/mocking.py | 7 ++++++- tox.ini | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 253924d0..59986c60 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ license.text = "Apache-2.0" keywords = ["juju", "test"] dependencies = [ - "ops~=2.17", + "ops~=2.15", "PyYAML>=6.0.1", ] readme = "README.md" diff --git a/scenario/context.py b/scenario/context.py index 4f4e409b..26665c06 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -27,7 +27,10 @@ ) if TYPE_CHECKING: # pragma: no cover - from ops._private.harness import ExecArgs + try: + from ops._private.harness import ExecArgs # type: ignore + except ImportError: + from ops.testing import ExecArgs # type: ignore from scenario.ops_main_mock import Ops from scenario.state import ( @@ -499,7 +502,7 @@ def __init__( self.charm_root = charm_root self.juju_version = juju_version if juju_version.split(".")[0] == "2": - logger.warn( + logger.warning( "Juju 2.x is closed and unsupported. You may encounter inconsistencies.", ) diff --git a/scenario/mocking.py b/scenario/mocking.py index 99b5d768..d3ec6e20 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -22,7 +22,12 @@ ) from ops import JujuVersion, pebble -from ops._private.harness import ExecArgs, _TestingPebbleClient + +try: + from ops._private.harness import ExecArgs, _TestingPebbleClient # type: ignore +except ImportError: + from ops.testing import ExecArgs, _TestingPebbleClient # type: ignore + from ops.model import CloudSpec as CloudSpec_Ops from ops.model import ModelError from ops.model import Port as Port_Ops diff --git a/tox.ini b/tox.ini index bfe802f1..d3155731 100644 --- a/tox.ini +++ b/tox.ini @@ -43,7 +43,7 @@ commands = description = Static typing checks. skip_install = true deps = - ops~=2.17 + ops~=2.15 pyright==1.1.347 commands = pyright scenario