Merge branch 'main' into add-scenario-benchmark-tests

.github/workflows/framework-tests.yaml

@@ -43,6 +43,7 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     strategy:
+      fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest]
         python-version: ["3.8", "3.10", "3.12"]
@@ -66,6 +67,7 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     strategy:
+      fail-fast: false
       matrix:
         os: [ubuntu-latest]
         python-version: ["3.8", "3.10", "3.12"]
@@ -107,6 +109,7 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     strategy:
+      fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest]
         python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]

ops/_private/harness.py

@@ -2271,13 +2271,13 @@ def _config_set(self, key: str, value: Union[str, int, float, bool]):
         declared_type = option.get('type')
         if not declared_type:
             raise RuntimeError(
-                f'Incorrectly formatted `options.yaml`, option {key} '
+                f'Incorrectly formatted config in YAML, option {key} '
                 'is expected to declare a `type`.'
             )
 
         if declared_type not in self._supported_types:
             raise RuntimeError(
-                'Incorrectly formatted `options.yaml`: `type` needs to be one '
+                'Incorrectly formatted config in YAML: `type` needs to be one '
                 'of [{}], not {}.'.format(', '.join(self._supported_types), declared_type)
             )
 

test/test_main.py

@@ -189,12 +189,6 @@ def test_storage_no_storage(self):
 
     def test_storage_with_storage(self):
         # here we patch juju_backend_available, so it gets set up and falls over when used
-        with patch('ops.storage.juju_backend_available') as juju_backend_available:
-            juju_backend_available.return_value = True
-            with pytest.raises(FileNotFoundError, match='state-get'):
-                self._check(ops.CharmBase, use_juju_for_storage=True)
-
-    def test_controller_storage_deprecated(self):
         with patch('ops.storage.juju_backend_available') as juju_backend_available:
             juju_backend_available.return_value = True
             with pytest.warns(DeprecationWarning, match='Controller storage'):

test/test_model.py

@@ -3643,11 +3643,13 @@ def test_from_dict(self):
         assert info.rotates is None
         assert info.description is None
 
-        info = ops.SecretInfo.from_dict('5', {'revision': 9})
+        with pytest.warns(DeprecationWarning, match='`model_uuid` should always be provided'):
+            info = ops.SecretInfo.from_dict('5', {'revision': 9})
         assert info.id == 'secret:5'
         assert info.revision == 9
 
-        info = ops.SecretInfo.from_dict('secret://abcd/6', {'revision': 9})
+        with pytest.warns(DeprecationWarning, match='`model_uuid` should always be provided'):
+            info = ops.SecretInfo.from_dict('secret://abcd/6', {'revision': 9})
         assert info.id == 'secret://abcd/6'
         assert info.revision == 9
 

testing/src/scenario/_runtime.py

@@ -224,7 +224,9 @@ def _virtual_charm_root(self):
         actions_yaml = virtual_charm_root / "actions.yaml"
 
         metadata_files_present: Dict[Path, Optional[str]] = {
-            file: file.read_text() if file.exists() else None
+            file: file.read_text()
+            if charm_virtual_root_is_custom and file.exists()
+            else None
             for file in (metadata_yaml, config_yaml, actions_yaml)
         }
 

testing/src/scenario/mocking.py

@@ -481,6 +481,7 @@ def secret_info_get(
             expires=secret.expire,
             rotation=secret.rotate,
             rotates=None,  # not implemented yet.
+            model_uuid=self._state.model.uuid,
         )
 
     def secret_set(

testing/src/scenario/state.py

@@ -122,26 +122,43 @@ class _MaxPositionalArgs:
 
         _max_positional_args = n
 
-        def __new__(cls, *args: Any, **kwargs: Any):
+        @classmethod
+        def _annotate_class(cls):
+            """Record information about which parameters are positional vs. keyword-only."""
+            if hasattr(cls, "_init_parameters"):
+                # We don't support dynamically changing the signature of a
+                # class, so we assume here it's the same as previously.
+                # In addition, the class and the function that provides it
+                # are private, so we generally don't expect anyone to be
+                # doing anything radical with these.
+                return
             # inspect.signature guarantees the order of parameters is as
             # declared, which aligns with dataclasses. Simpler ways of
             # getting the arguments (like __annotations__) do not have that
             # guarantee, although in practice it is the case.
-            parameters = inspect.signature(cls.__init__).parameters
-            required_args = [
+            cls._init_parameters = parameters = inspect.signature(
+                cls.__init__
+            ).parameters
+            cls._init_kw_only = {
+                name
+                for name in tuple(parameters)[cls._max_positional_args :]
+                if not name.startswith("_")
+            }
+            cls._init_required_args = [
                 name
                 for name in tuple(parameters)
-                if parameters[name].default is inspect.Parameter.empty
-                and name not in kwargs
-                and name != "self"
+                if name != "self"
+                and parameters[name].default is inspect.Parameter.empty
+            ]
+
+        def __new__(cls, *args: Any, **kwargs: Any):
+            cls._annotate_class()
+            required_args = [
+                name for name in cls._init_required_args if name not in kwargs
             ]
             n_posargs = len(args)
             max_n_posargs = cls._max_positional_args
-            kw_only = {
-                name
-                for name in tuple(parameters)[max_n_posargs:]
-                if not name.startswith("_")
-            }
+            kw_only = cls._init_kw_only
             if n_posargs > max_n_posargs:
                 raise TypeError(
                     f"{cls.__name__} takes {max_n_posargs} positional "
@@ -180,6 +197,10 @@ def __reduce__(self):
     return _MaxPositionalArgs
 
 
+# A lot of JujuLogLine objects are created, so we want them to be fast and light.
+# Dataclasses define __slots__, so are small, and a namedtuple is actually
+# slower to create than a dataclass. A plain dictionary (or TypedDict) would be
+# about twice as fast, but less convenient to use.
 @dataclasses.dataclass(frozen=True)
 class JujuLogLine:
     """An entry in the Juju debug-log."""

testing/tests/test_context_on.py

@@ -56,8 +56,6 @@ def _on_event(self, event: ops.EventBase):
         ("update_status", ops.UpdateStatusEvent),
         ("config_changed", ops.ConfigChangedEvent),
         ("upgrade_charm", ops.UpgradeCharmEvent),
-        ("pre_series_upgrade", ops.PreSeriesUpgradeEvent),
-        ("post_series_upgrade", ops.PostSeriesUpgradeEvent),
         ("leader_elected", ops.LeaderElectedEvent),
     ],
 )
@@ -72,6 +70,25 @@ def test_simple_events(event_name: str, event_kind: typing.Type[ops.EventBase]):
         assert isinstance(mgr.charm.observed[0], event_kind)
 
 
+@pytest.mark.parametrize(
+    "event_name, event_kind",
+    [
+        ("pre_series_upgrade", ops.PreSeriesUpgradeEvent),
+        ("post_series_upgrade", ops.PostSeriesUpgradeEvent),
+    ],
+)
+def test_simple_deprecated_events(event_name, event_kind):
+    ctx = scenario.Context(ContextCharm, meta=META, actions=ACTIONS)
+    # These look like:
+    #   ctx.run(ctx.on.pre_series_upgrade(), state)
+    with pytest.warns(DeprecationWarning):
+        with ctx(getattr(ctx.on, event_name)(), scenario.State()) as mgr:
+            mgr.run()
+            assert len(mgr.charm.observed) == 2
+            assert isinstance(mgr.charm.observed[1], ops.CollectStatusEvent)
+            assert isinstance(mgr.charm.observed[0], event_kind)
+
+
 @pytest.mark.parametrize("as_kwarg", [True, False])
 @pytest.mark.parametrize(
     "event_name,event_kind,owner",

tox.ini

@@ -104,7 +104,9 @@ deps =
     -e .
     -e testing
 commands =
-    pytest -n auto --ignore={[vars]tst_path}smoke --ignore={[vars]tst_path}test_benchmark.py -v --tb native {posargs}
+    pytest -n auto --ignore={[vars]tst_path}smoke --ignore={[vars]tst_path}test_benchmark.py \
+        -v --tb native \
+        -W 'ignore:Harness is deprecated:PendingDeprecationWarning' {posargs}
 
 [testenv:coverage]
 description = Run unit tests with coverage
@@ -125,7 +127,8 @@ commands =
     mkdir -p .report
     coverage run --source={[vars]src_path},testing/src/scenario \
              -m pytest --ignore={[vars]tst_path}smoke --ignore={[vars]tst_path}test_benchmark.py \
-             -v --tb native {posargs}
+             -v --tb native \
+             -W 'ignore:Harness is deprecated:PendingDeprecationWarning' {posargs}
     coverage xml -o .report/coverage.xml
     coverage report