refactor: fix the testing src-layout structure and use relative impor…

…ts (#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 <[email protected]>

.gitignore

@@ -21,7 +21,7 @@ coverage.xml
 /dist
 /build
 /docs/_build
-/testing/ops_scenario.egg-info/
+ops_scenario.egg-info
 /testing/build/
 /testing/dist/
 

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 = "[email protected]" }
@@ -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"

testing/src/__init__.py → 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,

testing/src/_consistency_checker.py → 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")
 

testing/src/context.py → 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,

testing/src/mocking.py → 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,

testing/src/ops_main_mock.py → 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,

testing/src/runtime.py → 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
     >>>

testing/src/state.py → 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))
 

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 =