From 53f1bb4ea3697af8e00a509052997b840e91229d Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 14 Oct 2024 12:37:25 +1300 Subject: [PATCH] refactor: fix the testing src-layout structure and use relative imports (#1431) The original commit merging ops-scenario into this repository had an error in the [src-layout](https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/) structure: the `src` folder should contain a folder for each of the importable packages, not have all of the individual modules. This PR moves everything in `testing/src/` to `testing/src/scenario/` to fix this. This means that the build tool can find the package without any help, and makes editable installs work correctly, which removes a quirk in the tox dependency installs. It also makes it simpler to work on the project, since the `ops` and `scenario` packages are available in the expected format with the expected name (although src-layout encourages editable installs for development). For the second part of the cleanup, the PR changes the `testing/src/scenario` imports to be relative for everything inside of that location. While adjusting these, most of the imports are adjusted to import from the top-level `ops` namespace rather than from individual modules (except for non-exported names). --------- Co-authored-by: Dima Tisnek --- .gitignore | 2 +- testing/pyproject.toml | 5 +--- testing/src/{ => scenario}/__init__.py | 6 ++-- .../{ => scenario}/_consistency_checker.py | 8 ++--- testing/src/{ => scenario}/context.py | 12 ++++---- testing/src/{ => scenario}/errors.py | 0 testing/src/{ => scenario}/logger.py | 0 testing/src/{ => scenario}/mocking.py | 29 ++++++++++--------- testing/src/{ => scenario}/ops_main_mock.py | 8 ++--- testing/src/{ => scenario}/py.typed | 0 testing/src/{ => scenario}/runtime.py | 23 ++++++++------- testing/src/{ => scenario}/state.py | 17 ++++------- tox.ini | 24 +++------------ 13 files changed, 57 insertions(+), 77 deletions(-) rename testing/src/{ => scenario}/__init__.py (96%) rename testing/src/{ => scenario}/_consistency_checker.py (99%) rename testing/src/{ => scenario}/context.py (99%) rename testing/src/{ => scenario}/errors.py (100%) rename testing/src/{ => scenario}/logger.py (100%) rename testing/src/{ => scenario}/mocking.py (98%) rename testing/src/{ => scenario}/ops_main_mock.py (97%) rename testing/src/{ => scenario}/py.typed (100%) rename testing/src/{ => scenario}/runtime.py (97%) rename testing/src/{ => scenario}/state.py (99%) diff --git a/.gitignore b/.gitignore index 97e7a9657..17446841b 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,7 @@ coverage.xml /dist /build /docs/_build -/testing/ops_scenario.egg-info/ +ops_scenario.egg-info /testing/build/ /testing/dist/ diff --git a/testing/pyproject.toml b/testing/pyproject.toml index fd8eb83c6..eb5680a0d 100644 --- a/testing/pyproject.toml +++ b/testing/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "ops-scenario" -version = "7.0.5" +version = "7.0.6.dev0" authors = [ { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" } @@ -41,9 +41,6 @@ classifiers = [ "Homepage" = "https://github.com/canonical/operator" "Bug Tracker" = "https://github.com/canonical/operator/issues" -[tool.setuptools.package-dir] -scenario = "src" - [tool.ruff.format] # Like Black, use double quotes for strings. quote-style = "double" diff --git a/testing/src/__init__.py b/testing/src/scenario/__init__.py similarity index 96% rename from testing/src/__init__.py rename to testing/src/scenario/__init__.py index 557beb3d4..b4dff01df 100644 --- a/testing/src/__init__.py +++ b/testing/src/scenario/__init__.py @@ -60,10 +60,10 @@ def test_base(): assert out.unit_status == UnknownStatus() """ -from scenario.context import CharmEvents, Context, Manager -from scenario.errors import StateValidationError # For backwards compatibility. +from .context import CharmEvents, Context, Manager +from .errors import StateValidationError # For backwards compatibility. from ops._private.harness import ActionFailed # For backwards compatibility. -from scenario.state import ( +from .state import ( ActiveStatus, Address, AnyJson, diff --git a/testing/src/_consistency_checker.py b/testing/src/scenario/_consistency_checker.py similarity index 99% rename from testing/src/_consistency_checker.py rename to testing/src/scenario/_consistency_checker.py index eab68c416..81df133d1 100644 --- a/testing/src/_consistency_checker.py +++ b/testing/src/scenario/_consistency_checker.py @@ -38,9 +38,9 @@ Union, ) -from scenario.errors import InconsistentScenarioError -from scenario.runtime import logger as scenario_logger -from scenario.state import ( +from .errors import InconsistentScenarioError +from .runtime import logger as scenario_logger +from .state import ( CharmType, PeerRelation, SubordinateRelation, @@ -50,7 +50,7 @@ ) if TYPE_CHECKING: # pragma: no cover - from scenario.state import State, _Event + from .state import State, _Event logger = scenario_logger.getChild("consistency_checker") diff --git a/testing/src/context.py b/testing/src/scenario/context.py similarity index 99% rename from testing/src/context.py rename to testing/src/scenario/context.py index 13e8a6132..ef0ffca9a 100644 --- a/testing/src/context.py +++ b/testing/src/scenario/context.py @@ -19,14 +19,14 @@ import ops from ops._private.harness import ActionFailed -from scenario.errors import ( +from .errors import ( AlreadyEmittedError, ContextSetupError, MetadataNotFoundError, ) -from scenario.logger import logger as scenario_logger -from scenario.runtime import Runtime -from scenario.state import ( +from .logger import logger as scenario_logger +from .runtime import Runtime +from .state import ( CharmType, CheckInfo, Container, @@ -40,8 +40,8 @@ if TYPE_CHECKING: # pragma: no cover from ops._private.harness import ExecArgs - from scenario.ops_main_mock import Ops - from scenario.state import ( + from .ops_main_mock import Ops + from .state import ( AnyJson, CharmType, JujuLogLine, diff --git a/testing/src/errors.py b/testing/src/scenario/errors.py similarity index 100% rename from testing/src/errors.py rename to testing/src/scenario/errors.py diff --git a/testing/src/logger.py b/testing/src/scenario/logger.py similarity index 100% rename from testing/src/logger.py rename to testing/src/scenario/logger.py diff --git a/testing/src/mocking.py b/testing/src/scenario/mocking.py similarity index 98% rename from testing/src/mocking.py rename to testing/src/scenario/mocking.py index 97bfe97f1..754225715 100644 --- a/testing/src/mocking.py +++ b/testing/src/scenario/mocking.py @@ -23,26 +23,29 @@ get_args, ) -from ops import JujuVersion, pebble +from ops import ( + JujuVersion, + pebble, + SecretInfo, + SecretNotFoundError, + RelationNotFoundError, + SecretRotate, + ModelError, +) 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 -from ops.model import RelationNotFoundError from ops.model import Secret as Secret_Ops # lol from ops.model import ( - SecretInfo, - SecretNotFoundError, - SecretRotate, _format_action_result_dict, _ModelBackend, _SettableStatusName, ) from ops.pebble import Client, ExecError -from scenario.errors import ActionMissingFromContextError -from scenario.logger import logger as scenario_logger -from scenario.state import ( +from .errors import ActionMissingFromContextError +from .logger import logger as scenario_logger +from .state import ( CharmType, JujuLogLine, Mount, @@ -58,9 +61,9 @@ ) if TYPE_CHECKING: # pragma: no cover - from scenario.context import Context - from scenario.state import Container as ContainerSpec - from scenario.state import Exec, Secret, State, _CharmSpec, _Event + from .context import Context + from .state import Container as ContainerSpec + from .state import Exec, Secret, State, _CharmSpec, _Event logger = scenario_logger.getChild("mocking") @@ -393,7 +396,7 @@ def secret_add( rotate: Optional[SecretRotate] = None, owner: Optional[Literal["unit", "app"]] = None, ) -> str: - from scenario.state import Secret + from .state import Secret secret = Secret( content, diff --git a/testing/src/ops_main_mock.py b/testing/src/scenario/ops_main_mock.py similarity index 97% rename from testing/src/ops_main_mock.py rename to testing/src/scenario/ops_main_mock.py index 0355dc406..58bb1d05c 100644 --- a/testing/src/ops_main_mock.py +++ b/testing/src/scenario/ops_main_mock.py @@ -21,11 +21,11 @@ from ops.charm import CharmMeta from ops.log import setup_root_logging -from scenario.errors import BadOwnerPath, NoObserverError +from .errors import BadOwnerPath, NoObserverError if TYPE_CHECKING: # pragma: no cover - from scenario.context import Context - from scenario.state import CharmType, State, _CharmSpec, _Event + from .context import Context + from .state import CharmType, State, _CharmSpec, _Event # pyright: reportPrivateUsage=false @@ -98,7 +98,7 @@ def setup_framework( context: "Context", charm_spec: "_CharmSpec[CharmType]", ): - from scenario.mocking import _MockModelBackend + from .mocking import _MockModelBackend model_backend = _MockModelBackend( state=state, diff --git a/testing/src/py.typed b/testing/src/scenario/py.typed similarity index 100% rename from testing/src/py.typed rename to testing/src/scenario/py.typed diff --git a/testing/src/runtime.py b/testing/src/scenario/runtime.py similarity index 97% rename from testing/src/runtime.py rename to testing/src/scenario/runtime.py index 613258e4a..1206c119c 100644 --- a/testing/src/runtime.py +++ b/testing/src/scenario/runtime.py @@ -25,22 +25,23 @@ ) import yaml -from ops import CollectStatusEvent, pebble -from ops.framework import ( +from ops import ( + CollectStatusEvent, + pebble, CommitEvent, EventBase, Framework, Handle, NoTypeError, PreCommitEvent, - _event_regex, ) from ops.storage import NoSnapshotError, SQLiteStorage +from ops.framework import _event_regex from ops._private.harness import ActionFailed -from scenario.errors import NoObserverError, UncaughtCharmError -from scenario.logger import logger as scenario_logger -from scenario.state import ( +from .errors import NoObserverError, UncaughtCharmError +from .logger import logger as scenario_logger +from .state import ( DeferredEvent, PeerRelation, Relation, @@ -49,8 +50,8 @@ ) if TYPE_CHECKING: # pragma: no cover - from scenario.context import Context - from scenario.state import CharmType, State, _CharmSpec, _Event + from .context import Context + from .state import CharmType, State, _CharmSpec, _Event logger = scenario_logger.getChild("runtime") STORED_STATE_REGEX = re.compile( @@ -435,7 +436,7 @@ def exec( # todo consider forking out a real subprocess and do the mocking by # mocking hook tool executables - from scenario._consistency_checker import check_consistency # avoid cycles + from ._consistency_checker import check_consistency # avoid cycles check_consistency(state, event, self._charm_spec, self._juju_version) @@ -459,7 +460,7 @@ def exec( os.environ.update(env) logger.info(" - Entering ops.main (mocked).") - from scenario.ops_main_mock import Ops # noqa: F811 + from .ops_main_mock import Ops # noqa: F811 try: ops = Ops( @@ -513,7 +514,7 @@ def _capture_events( Arguments exposed so that you can define your own fixtures if you want to. Example:: - >>> from ops.charm import StartEvent + >>> from ops import StartEvent >>> from scenario import Event, State >>> from charm import MyCustomEvent, MyCharm # noqa >>> diff --git a/testing/src/state.py b/testing/src/scenario/state.py similarity index 99% rename from testing/src/state.py rename to testing/src/scenario/state.py index 8e46afe1f..29489ba3f 100644 --- a/testing/src/state.py +++ b/testing/src/scenario/state.py @@ -37,17 +37,15 @@ import ops import yaml -from ops import pebble -from ops.charm import CharmBase, CharmEvents -from ops.model import CloudCredential as CloudCredential_Ops -from ops.model import CloudSpec as CloudSpec_Ops -from ops.model import SecretRotate, StatusBase +from ops import pebble, CharmBase, CharmEvents, SecretRotate, StatusBase +from ops import CloudCredential as CloudCredential_Ops +from ops import CloudSpec as CloudSpec_Ops -from scenario.errors import MetadataNotFoundError, StateValidationError -from scenario.logger import logger as scenario_logger +from .errors import MetadataNotFoundError, StateValidationError +from .logger import logger as scenario_logger if TYPE_CHECKING: # pragma: no cover - from scenario import Context + from . import Context AnyJson = Union[str, bool, Dict[str, "AnyJson"], int, float, List["AnyJson"]] RawSecretRevisionContents = RawDataBagContents = Dict[str, str] @@ -675,9 +673,6 @@ def _get_databag_for_remote(self, unit_id: UnitID) -> RawDataBagContents: def _random_model_name(): - import random - import string - space = string.ascii_letters + string.digits return "".join(random.choice(space) for _ in range(20)) diff --git a/tox.ini b/tox.ini index 4b9ffbb63..7854dfed1 100644 --- a/tox.ini +++ b/tox.ini @@ -84,11 +84,7 @@ deps = pytest~=7.2 typing_extensions~=4.2 -e . - # TODO: If the code in testing changes, it isn't reinstalled. - # -e doesn't install it with the right name. We should probably - # build a wheel and then use that across the environments, - # similar to what was done in the ops-scenario repository. - ./testing + -e testing commands = pyright {posargs} @@ -105,11 +101,7 @@ deps = typing_extensions~=4.2 jsonpatch~=1.33 -e . - # TODO: If the code in testing changes, it isn't reinstalled. - # -e doesn't install it with the right name. We should probably - # build a wheel and then use that across the environments, - # similar to what was done in the ops-scenario repository. - ./testing + -e testing commands = pytest -n auto --ignore={[vars]tst_path}smoke -v --tb native {posargs} @@ -126,11 +118,7 @@ deps = typing_extensions~=4.2 jsonpatch~=1.33 -e . - # TODO: If the code in testing changes, it isn't reinstalled. - # -e doesn't install it with the right name. We should probably - # build a wheel and then use that across the environments, - # similar to what was done in the ops-scenario repository. - ./testing + -e testing commands = coverage run --source={[vars]src_path} \ -m pytest --ignore={[vars]tst_path}smoke -v --tb native {posargs} @@ -185,11 +173,7 @@ allowlist_externals = cp deps = -e . - # TODO: If the code in testing changes, it isn't reinstalled. - # -e doesn't install it with the right name. We should probably - # build a wheel and then use that across the environments, - # similar to what was done in the ops-scenario repository. - ./testing + -e testing pytest pytest-markdown-docs commands =