Skip to content

Commit

Permalink
pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
PietroPasotti committed Sep 19, 2023
1 parent 84ee6d6 commit 54c9c8a
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 36 deletions.
13 changes: 8 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1032,10 +1032,10 @@ can't emit multiple events in a single charm execution.

# The virtual charm root

Before executing the charm, Scenario writes the metadata, config, and actions `yaml`s to a temporary directory. The
Before executing the charm, Scenario copies the charm's `/src`, any libs, the metadata, config, and actions `yaml`s to a temporary directory. The
charm will see that tempdir as its 'root'. This allows us to keep things simple when dealing with metadata that can be
either inferred from the charm type being passed to `Context` or be passed to it as an argument, thereby overriding
the inferred one. This also allows you to test with charms defined on the fly, as in:
the inferred one. This also allows you to test charms defined on the fly, as in:

```python
from ops.charm import CharmBase
Expand All @@ -1052,7 +1052,7 @@ ctx.run('start', State())
```

A consequence of this fact is that you have no direct control over the tempdir that we are creating to put the metadata
you are passing to trigger (because `ops` expects it to be a file...). That is, unless you pass your own:
you are passing to `.run()` (because `ops` expects it to be a file...). That is, unless you pass your own:

```python
from ops.charm import CharmBase
Expand All @@ -1073,8 +1073,11 @@ state = Context(
```

Do this, and you will be able to set up said directory as you like before the charm is run, as well as verify its
contents after the charm has run. Do keep in mind that the metadata files will be overwritten by Scenario, and therefore
ignored.
contents after the charm has run. Do keep in mind that any metadata files you create in it will be overwritten by Scenario, and therefore
ignored, if you pass any metadata keys to `Context`. Omit `meta` in the call
above, and Scenario will instead attempt to read `metadata.yaml` from the
temporary directory.


# Immutability

Expand Down
45 changes: 25 additions & 20 deletions scenario/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,44 +283,49 @@ def _virtual_charm_root(self) -> typing.ContextManager[Path]:
# is what the user passed via the CharmSpec
spec = self._charm_spec

if vroot := self._charm_root:
vroot_is_custom = True
virtual_charm_root = Path(vroot)
if charm_virtual_root := self._charm_root:
charm_virtual_root_is_custom = True
virtual_charm_root = Path(charm_virtual_root)
else:
vroot = tempfile.TemporaryDirectory()
virtual_charm_root = Path(vroot.name)
vroot_is_custom = False
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, Union[str, False]] = {
file: file.read_text() if file.exists() else False
metadata_files_present: Dict[Path, Optional[str]] = {
file: file.read_text() if file.exists() else None
for file in (metadata_yaml, config_yaml, actions_yaml)
}

any_metadata_files_present_in_vroot = any(metadata_files_present.values())
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 vroot_is_custom:
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_vroot:
if any_metadata_files_present_in_charm_virtual_root:
logger.debug(
f"metadata files found in custom vroot {vroot}. "
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_vroot:
elif (
not spec.is_autoloaded and any_metadata_files_present_in_charm_virtual_root
):
logger.warn(
f"Some metadata files found in custom user-provided vroot {vroot} "
"while you have passed meta, config or actions to Context.run(). "
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(). "
"Vroot metadata files will be overwritten for the "
"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 vroot before calling run.",
"To avoid this, clean any metadata files from the charm_root before calling run.",
)

metadata_yaml.write_text(yaml.safe_dump(spec.meta))
Expand All @@ -329,15 +334,15 @@ def _virtual_charm_root(self) -> typing.ContextManager[Path]:

yield virtual_charm_root

if vroot_is_custom:
if charm_virtual_root_is_custom:
for file, previous_content in metadata_files_present.items():
if not previous_content: # False: file did not exist before
if previous_content is None: # None == file did not exist before
file.unlink()
else:
file.write_text(previous_content)

else:
vroot.cleanup()
charm_virtual_root.cleanup()

@staticmethod
def _get_state_db(temporary_charm_root: Path):
Expand Down
22 changes: 11 additions & 11 deletions tests/test_e2e/test_vroot.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ def __init__(self, framework: Framework):


@pytest.fixture
def vroot():
with tempfile.TemporaryDirectory() as myvroot:
t = Path(myvroot)
def charm_virtual_root():
with tempfile.TemporaryDirectory() as mycharm_virtual_root:
t = Path(mycharm_virtual_root)
src = t / "src"
src.mkdir()
foobar = src / "foo.bar"
Expand All @@ -39,23 +39,23 @@ def vroot():
yield t


def test_vroot(vroot):
def test_charm_virtual_root(charm_virtual_root):
out = trigger(
State(),
"start",
charm_type=MyCharm,
meta=MyCharm.META,
charm_root=vroot,
charm_root=charm_virtual_root,
)
assert out.unit_status == ("active", "hello world")


def test_vroot_cleanup_if_exists(vroot):
meta_file = vroot / "metadata.yaml"
def test_charm_virtual_root_cleanup_if_exists(charm_virtual_root):
meta_file = charm_virtual_root / "metadata.yaml"
raw_ori_meta = yaml.safe_dump({"name": "karl"})
meta_file.write_text(raw_ori_meta)

with Context(MyCharm, meta=MyCharm.META, charm_root=vroot).manager(
with Context(MyCharm, meta=MyCharm.META, charm_root=charm_virtual_root).manager(
"start",
State(),
) as mgr:
Expand All @@ -72,12 +72,12 @@ def test_vroot_cleanup_if_exists(vroot):
assert meta_file.exists()


def test_vroot_cleanup_if_not_exists(vroot):
meta_file = vroot / "metadata.yaml"
def test_charm_virtual_root_cleanup_if_not_exists(charm_virtual_root):
meta_file = charm_virtual_root / "metadata.yaml"

assert not meta_file.exists()

with Context(MyCharm, meta=MyCharm.META, charm_root=vroot).manager(
with Context(MyCharm, meta=MyCharm.META, charm_root=charm_virtual_root).manager(
"start",
State(),
) as mgr:
Expand Down

0 comments on commit 54c9c8a

Please sign in to comment.