diff --git a/testing/src/scenario/_ops_main_mock.py b/testing/src/scenario/_ops_main_mock.py index e760249d7..1be25a90b 100644 --- a/testing/src/scenario/_ops_main_mock.py +++ b/testing/src/scenario/_ops_main_mock.py @@ -148,13 +148,11 @@ def __init__( super().__init__(self.charm_spec.charm_type, model_backend, juju_context=juju_context) def _load_charm_meta(self): - metadata = (self._charm_root / 'metadata.yaml').read_text() - actions_meta = self._charm_root / 'actions.yaml' - actions_metadata = actions_meta.read_text() if actions_meta.exists() else None - config_meta = self._charm_root / 'config.yaml' - config_metadata = config_meta.read_text() if config_meta.exists() else None - - return ops.CharmMeta.from_yaml(metadata, actions_metadata, config_metadata) + return ops.CharmMeta( + self.charm_spec.meta, + self.charm_spec.actions or {}, + self.charm_spec.config or {}, + ) @property def _framework_class(self): diff --git a/testing/src/scenario/_runtime.py b/testing/src/scenario/_runtime.py index 07de01ec9..48c82a6ec 100644 --- a/testing/src/scenario/_runtime.py +++ b/testing/src/scenario/_runtime.py @@ -15,8 +15,6 @@ from pathlib import Path from typing import TYPE_CHECKING, Generic -import yaml - from ops import JujuContext, pebble from ops._main import _Abort from ops._private.harness import ActionFailed @@ -209,72 +207,12 @@ class WrappedCharm(charm_type): @contextmanager def _virtual_charm_root(self): - # If we are using runtime on a real charm, we can make some assumptions about the - # directory structure we are going to find. - # If we're, say, dynamically defining charm types and doing tests on them, we'll have to - # generate the metadata files ourselves. To be sure, we ALWAYS use a tempdir. Ground truth - # is what the user passed via the CharmSpec - spec = self._charm_spec - - if charm_virtual_root := self._charm_root: - charm_virtual_root_is_custom = True - virtual_charm_root = Path(charm_virtual_root) - else: - charm_virtual_root = tempfile.TemporaryDirectory() - virtual_charm_root = Path(charm_virtual_root.name) - charm_virtual_root_is_custom = False - - metadata_yaml = virtual_charm_root / 'metadata.yaml' - config_yaml = virtual_charm_root / 'config.yaml' - actions_yaml = virtual_charm_root / 'actions.yaml' - - metadata_files_present: dict[Path, str | 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) - } - - any_metadata_files_present_in_charm_virtual_root = any( - v is not None for v in metadata_files_present.values() - ) - - if spec.is_autoloaded and charm_virtual_root_is_custom: - # since the spec is autoloaded, in theory the metadata contents won't differ, so we can - # overwrite away even if the custom vroot is the real charm root (the local repo). - # Still, log it for clarity. - if any_metadata_files_present_in_charm_virtual_root: - logger.debug( - f'metadata files found in custom charm_root {charm_virtual_root}. ' - f'The spec was autoloaded so the contents should be identical. ' - f'Proceeding...', - ) - - elif not spec.is_autoloaded and any_metadata_files_present_in_charm_virtual_root: - logger.warning( - f'Some metadata files found in custom user-provided charm_root ' - f'{charm_virtual_root} while you have passed meta, config or actions to ' - f'Context.run(). ' - 'Single source of truth are the arguments passed to Context.run(). ' - 'charm_root metadata files will be overwritten for the ' - 'duration of this test, and restored afterwards. ' - 'To avoid this, clean any metadata files from the charm_root before calling run.', - ) - - metadata_yaml.write_text(yaml.safe_dump(spec.meta)) - config_yaml.write_text(yaml.safe_dump(spec.config or {})) - actions_yaml.write_text(yaml.safe_dump(spec.actions or {})) - - yield virtual_charm_root - - if charm_virtual_root_is_custom: - for file, previous_content in metadata_files_present.items(): - if previous_content is None: # None == file did not exist before - file.unlink() - else: - file.write_text(previous_content) - - else: - # charm_virtual_root is a tempdir - typing.cast('tempfile.TemporaryDirectory', charm_virtual_root).cleanup() # type: ignore + if self._charm_root: + yield Path(self._charm_root) + return + tmp_dir = tempfile.TemporaryDirectory() + yield Path(tmp_dir.name) + tmp_dir.cleanup() @contextmanager def exec( diff --git a/testing/src/scenario/state.py b/testing/src/scenario/state.py index 2081384ed..268012a03 100644 --- a/testing/src/scenario/state.py +++ b/testing/src/scenario/state.py @@ -8,6 +8,7 @@ import copy import dataclasses import datetime +import functools import inspect import pathlib import random @@ -1961,16 +1962,10 @@ def _load_metadata_legacy(charm_root: pathlib.Path): """Load metadata from charm projects created with Charmcraft < 2.5.""" # back in the days, we used to have separate metadata.yaml, config.yaml and actions.yaml # files for charm metadata. - metadata_path = charm_root / 'metadata.yaml' - meta: dict[str, Any] = ( - yaml.safe_load(metadata_path.open()) if metadata_path.exists() else {} - ) - - config_path = charm_root / 'config.yaml' - config = yaml.safe_load(config_path.open()) if config_path.exists() else None - - actions_path = charm_root / 'actions.yaml' - actions = yaml.safe_load(actions_path.open()) if actions_path.exists() else None + charm_root = charm_root.absolute() + meta: dict[str, Any] = _load_yaml(charm_root / 'metadata.yaml') or {} + config = _load_yaml(charm_root / 'config.yaml') + actions = _load_yaml(charm_root / 'actions.yaml') return meta, config, actions @staticmethod @@ -1983,9 +1978,7 @@ def _load_metadata(charm_root: pathlib.Path): what ``charmcraft expand-extensions`` does at pack time. """ metadata_path = charm_root / 'charmcraft.yaml' - meta: dict[str, Any] = ( - yaml.safe_load(metadata_path.open()) if metadata_path.exists() else {} - ) + meta: dict[str, Any] = _load_yaml(metadata_path) or {} if not _is_valid_charmcraft_25_metadata(meta): meta = {} @@ -1994,8 +1987,13 @@ def _load_metadata(charm_root: pathlib.Path): if extensions: _apply_extensions(meta, extensions) - config = meta.pop('config', None) - actions = meta.pop('actions', None) + if 'config' in meta or 'actions' in meta: + meta = {**meta} + config = meta.pop('config', None) + actions = meta.pop('actions', None) + else: + config = None + actions = None return meta, config, actions @staticmethod @@ -2451,3 +2449,10 @@ def test_backup_action(): Every action invocation is automatically assigned a new one. Override in the rare cases where a specific ID is required.""" + + +@functools.lru_cache +def _load_yaml(path: pathlib.Path) -> Any | None: + if path.exists(): + return yaml.safe_load(path.read_text()) + return None diff --git a/testing/tests/test_e2e/test_vroot.py b/testing/tests/test_e2e/test_vroot.py index e5ee12a96..4657c9ab2 100644 --- a/testing/tests/test_e2e/test_vroot.py +++ b/testing/tests/test_e2e/test_vroot.py @@ -69,15 +69,11 @@ def test_charm_virtual_root_cleanup_if_exists(charm_virtual_root): State(), ) as mgr: assert meta_file.exists() - assert meta_file.read_text() == yaml.safe_dump({'name': 'my-charm'}) + assert meta_file.read_text() == raw_ori_meta # we don't write metadata to temp dir assert mgr.charm.meta.name == 'my-charm' # not karl! Context.meta takes precedence mgr.run() assert meta_file.exists() - # meta file was restored to its previous contents - assert meta_file.read_text() == raw_ori_meta - assert meta_file.exists() - def test_charm_virtual_root_cleanup_if_not_exists(charm_virtual_root): meta_file = charm_virtual_root / 'metadata.yaml' @@ -89,8 +85,7 @@ def test_charm_virtual_root_cleanup_if_not_exists(charm_virtual_root): ctx.on.start(), State(), ) as mgr: - assert meta_file.exists() - assert meta_file.read_text() == yaml.safe_dump({'name': 'my-charm'}) + assert not meta_file.exists() # we don't write metadata to temp dir mgr.run() assert not meta_file.exists()