refactor!: use ops._main._Manager in Scenario

.readthedocs.yaml

@@ -2,10 +2,7 @@ version: 2
 
 python:
   install:
-    - method: pip
-      path: .
-      extra_requirements:
-        - docs
+    - requirements: docs/requirements.txt
 
 build:
   os: ubuntu-22.04

docs/custom_conf.py

@@ -212,9 +212,7 @@
 # pyspelling, sphinx, sphinx-autobuild, sphinx-copybutton, sphinx-design,
 # sphinx-notfound-page, sphinx-reredirects, sphinx-tabs, sphinxcontrib-jquery,
 # sphinxext-opengraph
-custom_required_modules = [
-    'ops-scenario>=7.0.5,<8',
-]
+custom_required_modules = []
 
 # Add files or directories that should be excluded from processing.
 custom_excludes = [
@@ -318,6 +316,7 @@
     ('py:class', '_FileInfoDict'),
     ('py:class', '_NoticeDict'),
     ('py:class', '_ProgressDict'),
+    ('py:class', '_RawPortProtocolLiteral'),
     ('py:class', '_Readable'),
     ('py:class', '_RelationMetaDict'),
     ('py:class', '_ResourceMetaDict'),
@@ -347,6 +346,7 @@
     ('py:class', 'ops.testing._ConfigOption'),
     ('py:class', 'ops.testing.CharmType'),
     ('py:obj', 'ops.testing.CharmType'),
+    ('py:class', 'scenario.state.CharmType'),
     ('py:class', 'scenario.state._EntityStatus'),
     ('py:class', 'scenario.state._Event'),
     ('py:class', 'scenario.state._max_posargs.<locals>._MaxPositionalArgs'),

docs/requirements.txt

@@ -73,10 +73,6 @@ mdurl==0.1.2
     # via markdown-it-py
 myst-parser==4.0.0
     # via ops (pyproject.toml)
-ops==2.16.1
-    # via ops-scenario
-ops-scenario==7.0.5
-    # via ops (pyproject.toml)
 packaging==24.1
     # via sphinx
 pygments==2.18.0
@@ -89,9 +85,7 @@ pyspelling==2.10
 pyyaml==6.0.2
     # via
     #   myst-parser
-    #   ops
     #   ops (pyproject.toml)
-    #   ops-scenario
     #   pyspelling
 requests==2.32.3
     # via
@@ -166,8 +160,7 @@ wcmatch==9.0
 webencodings==0.5.1
     # via html5lib
 websocket-client==1.8.0
-    # via
-    #   ops
-    #   ops (pyproject.toml)
+    # via ops (pyproject.toml)
 websockets==12.0
     # via sphinx-autobuild
+./testing/

ops/_main.py

@@ -112,28 +112,6 @@ def _setup_event_links(charm_dir: Path, charm: 'ops.charm.CharmBase', juju_conte
             _create_event_link(charm, bound_event, link_to)
 
 
-def _emit_charm_event(charm: 'ops.charm.CharmBase', event_name: str, juju_context: _JujuContext):
-    """Emits a charm event based on a Juju event name.
-
-    Args:
-        charm: A charm instance to emit an event from.
-        event_name: A Juju event name to emit on a charm.
-        juju_context: An instance of the _JujuContext class.
-    """
-    event_to_emit = None
-    try:
-        event_to_emit = getattr(charm.on, event_name)
-    except AttributeError:
-        logger.debug('Event %s not defined for %s.', event_name, charm)
-
-    # If the event is not supported by the charm implementation, do
-    # not error out or try to emit it. This is to support rollbacks.
-    if event_to_emit is not None:
-        args, kwargs = _get_event_args(charm, event_to_emit, juju_context)
-        logger.debug('Emitting Juju event %s.', event_name)
-        event_to_emit.emit(*args, **kwargs)
-
-
 def _get_event_args(
     charm: 'ops.charm.CharmBase',
     bound_event: 'ops.framework.BoundEvent',
@@ -401,8 +379,11 @@ def __init__(
         model_backend: Optional[ops.model._ModelBackend] = None,
         use_juju_for_storage: Optional[bool] = None,
         charm_state_path: str = CHARM_STATE_FILE,
+        juju_context: Optional[_JujuContext] = None,
     ):
-        self._juju_context = _JujuContext.from_dict(os.environ)
+        if juju_context is None:
+            juju_context = _JujuContext.from_dict(os.environ)
+        self._juju_context = juju_context
         self._charm_state_path = charm_state_path
         self._charm_class = charm_class
         if model_backend is None:
@@ -413,7 +394,7 @@ def __init__(
         self._setup_root_logging()
 
         self._charm_root = self._juju_context.charm_dir
-        self._charm_meta = CharmMeta.from_charm_root(self._charm_root)
+        self._charm_meta = self._load_charm_meta()
         self._use_juju_for_storage = use_juju_for_storage
 
         # Set up dispatcher, framework and charm objects.
@@ -423,6 +404,9 @@ def __init__(
         self.framework = self._make_framework(self.dispatcher)
         self.charm = self._make_charm(self.framework, self.dispatcher)
 
+    def _load_charm_meta(self):
+        return CharmMeta.from_charm_root(self._charm_root)
+
     def _make_charm(self, framework: 'ops.framework.Framework', dispatcher: _Dispatcher):
         charm = self._charm_class(framework)
         dispatcher.ensure_event_links(charm)
@@ -482,7 +466,7 @@ def _make_framework(self, dispatcher: _Dispatcher):
         # If we are in a RelationBroken event, we want to know which relation is
         # broken within the model, not only in the event's `.relation` attribute.
 
-        if self._juju_context.dispatch_path.endswith('-relation-broken'):
+        if self._juju_context.dispatch_path.endswith(('-relation-broken', '_relation_broken')):
             broken_relation_id = self._juju_context.relation_id
         else:
             broken_relation_id = None
@@ -515,19 +499,48 @@ def _emit(self):
             self.framework.reemit()
 
         # Emit the Juju event.
-        _emit_charm_event(self.charm, self.dispatcher.event_name, self._juju_context)
+        self._emit_charm_event(self.dispatcher.event_name)
         # Emit collect-status events.
         ops.charm._evaluate_status(self.charm)
 
+    def _get_event_to_emit(self, event_name: str) -> Optional[ops.framework.BoundEvent]:
+        try:
+            return getattr(self.charm.on, event_name)
+        except AttributeError:
+            logger.debug('Event %s not defined for %s.', event_name, self.charm)
+        # If the event is not defined, return None.
+
+    def _emit_charm_event(self, event_name: str):
+        """Emits a charm event based on a Juju event name.
+
+        Args:
+            charm: A charm instance to emit an event from.
+            event_name: A Juju event name to emit on a charm.
+            juju_context: An instance of the _JujuContext class.
+        """
+        event_to_emit = self._get_event_to_emit(event_name)
+
+        # If the event is not supported by the charm implementation, do
+        # not error out or try to emit it. This is to support rollbacks.
+        if event_to_emit is not None:
+            args, kwargs = _get_event_args(self.charm, event_to_emit, self._juju_context)
+            logger.debug('Emitting Juju event %s.', event_name)
+            event_to_emit.emit(*args, **kwargs)
+
     def _commit(self):
         """Commit the framework and gracefully teardown."""
         self.framework.commit()
 
+    def _close(self):
+        """Perform any necessary cleanup before the framework is closed."""
+        # Provided for child classes - nothing needs to be done in the base.
+
     def run(self):
         """Emit and then commit the framework."""
         try:
             self._emit()
             self._commit()
+            self._close()
         finally:
             self.framework.close()
 

ops/testing.py

@@ -177,8 +177,8 @@
         # monkeypatch it in, so that the ops.testing.ActionFailed object is the
         # one that we expect, even if people are mixing Harness and Scenario.
         # https://github.com/canonical/ops-scenario/issues/201
+        import scenario._runtime as _runtime
         import scenario.context as _context
-        import scenario.runtime as _runtime
 
         _context.ActionFailed = ActionFailed  # type: ignore[reportPrivateImportUsage]
         _runtime.ActionFailed = ActionFailed  # type: ignore[reportPrivateImportUsage]

pyproject.toml

@@ -36,7 +36,6 @@ docs = [
     "sphinx-tabs",
     "sphinxcontrib-jquery",
     "sphinxext-opengraph",
-    "ops-scenario>=7.0.5,<8",
 ]
 testing = [
     "ops-scenario>=7.0.5,<8",

test/test_main.py

@@ -97,7 +97,7 @@ def __init__(
 
 
 @patch('ops._main.setup_root_logging', new=lambda *a, **kw: None)  # type: ignore
-@patch('ops._main._emit_charm_event', new=lambda *a, **kw: None)  # type: ignore
+@patch('ops._main._Manager._emit_charm_event', new=lambda *a, **kw: None)  # type: ignore
 @patch('ops.charm._evaluate_status', new=lambda *a, **kw: None)  # type: ignore
 class TestCharmInit:
     @patch('sys.stderr', new_callable=io.StringIO)
@@ -235,11 +235,11 @@ def __init__(self, framework: ops.Framework):
                 dispatch.chmod(0o755)
 
             with patch.dict(os.environ, fake_environ):
-                with patch('ops._main._emit_charm_event') as mock_charm_event:
+                with patch('ops._main._Manager._emit_charm_event') as mock_charm_event:
                     ops.main(MyCharm)
 
         assert mock_charm_event.call_count == 1
-        return mock_charm_event.call_args[0][1]
+        return mock_charm_event.call_args[0][0]
 
     def test_most_legacy(self):
         """Without dispatch, sys.argv[0] is used."""

test/test_main_invocation.py

@@ -24,7 +24,7 @@
 @pytest.fixture
 def charm_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
     monkeypatch.setattr('sys.argv', ('hooks/install',))
-    monkeypatch.setattr('ops._main._emit_charm_event', Mock())
+    monkeypatch.setattr('ops._main._Manager._emit_charm_event', Mock())
     monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock())
     monkeypatch.setattr('ops.charm._evaluate_status', Mock())
     monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path))

testing/src/scenario/_consistency_checker.py

@@ -39,7 +39,7 @@
 )
 
 from .errors import InconsistentScenarioError
-from .runtime import logger as scenario_logger
+from ._runtime import logger as scenario_logger
 from .state import (
     CharmType,
     PeerRelation,
@@ -170,16 +170,14 @@ def check_event_consistency(
     errors: List[str] = []
     warnings: List[str] = []
 
+    # custom event: can't make assumptions about its name and its semantics
+    # todo: should we then just skip the other checks?
     if not event._is_builtin_event(charm_spec):
-        # This is a custom event - we can't make assumptions about its name and
-        # semantics. It doesn't really make sense to do checks that are designed
-        # for relations, workloads, and so on - most likely those will end up
-        # with false positives. Realistically, we can't know about what the
-        # requirements for the custom event are (in terms of the state), so we
-        # skip everything here. Perhaps in the future, custom events could
-        # optionally include some sort of state metadata that made testing
-        # consistency possible?
-        return Results(errors, warnings)
+        warnings.append(
+            "this is a custom event; if its name makes it look like a builtin one "
+            "(e.g. a relation event, or a workload event), you might get some false-negative "
+            "consistency checks.",
+        )
 
     if event._is_relation_event:
         _check_relation_event(charm_spec, event, state, errors, warnings)