From d18e0ece75a7df8a1548bfb92c26e126b56b16be Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Sun, 7 Jun 2026 09:15:50 +1200 Subject: [PATCH 1/2] testing: chdir to charm root during event in Scenario Under Juju, the current working directory at the start of a hook is the charm root; under Scenario it was wherever pytest ran from. Charms (and libraries) that read files relative to the cwd therefore break only under Scenario. Mirror the existing JUJU_CHARM_DIR handling: save the cwd, chdir to temporary_charm_root after updating the env, and restore in finally before the virtual charm root tempdir is cleaned up. Closes #2045. --- testing/src/scenario/_runtime.py | 8 ++++++++ testing/tests/test_e2e/test_vroot.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/testing/src/scenario/_runtime.py b/testing/src/scenario/_runtime.py index d36d9a86d..8e281096f 100644 --- a/testing/src/scenario/_runtime.py +++ b/testing/src/scenario/_runtime.py @@ -321,6 +321,13 @@ def exec( previous_env = os.environ.copy() os.environ.update(env) + # In Juju, the current working directory is the charm root when a + # hook starts. Charms (and libraries) rely on this, e.g. to read + # files relative to it. Match that behaviour, restoring the + # original working directory once the event has been handled. + previous_cwd = os.getcwd() + os.chdir(temporary_charm_root) + logger.info(' - entering ops.main (mocked)') from ._ops_main_mock import Ops @@ -364,6 +371,7 @@ def exec( if key not in previous_env: del os.environ[key] os.environ.update(previous_env) + os.chdir(previous_cwd) logger.info(' - exited ops.main') logger.info('event dispatched. done.') diff --git a/testing/tests/test_e2e/test_vroot.py b/testing/tests/test_e2e/test_vroot.py index 54eecc86d..b48840f13 100644 --- a/testing/tests/test_e2e/test_vroot.py +++ b/testing/tests/test_e2e/test_vroot.py @@ -3,6 +3,7 @@ from __future__ import annotations +import os import tempfile from collections.abc import Generator, Mapping from pathlib import Path @@ -58,6 +59,26 @@ def test_charm_virtual_root(charm_virtual_root: Path): assert out.unit_status == ActiveStatus('hello world') +class CwdCharm(CharmBase): + META: Mapping[str, Any] = {'name': 'my-charm'} + + def __init__(self, framework: Framework): + super().__init__(framework) + # Charms may read files relative to the cwd, which Juju sets to the + # charm root. The cwd should match charm_dir during event handling. + self.unit.status = ActiveStatus(os.getcwd()) + + +def test_cwd_is_charm_root_during_event(): + cwd_before = os.getcwd() + ctx = Context(CwdCharm, meta=dict(CwdCharm.META)) + with ctx(ctx.on.start(), State()) as mgr: + mgr.run() + assert mgr.charm.unit.status.message == str(mgr.charm.framework.charm_dir) + # The original working directory is restored after the event is handled. + assert os.getcwd() == cwd_before + + def test_charm_virtual_root_cleanup_if_exists(charm_virtual_root: Path): meta_file = charm_virtual_root / 'metadata.yaml' raw_ori_meta = yaml.safe_dump({'name': 'karl'}) From c53e00ad08783ba2eeeb3fa5d99a7e4cf8609cf3 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 17 Jun 2026 10:53:45 +1200 Subject: [PATCH 2/2] fix: warn on cwd-relative reads instead of silently chdir'ing in Scenario Under Juju the working directory at hook start is the charm root; under Scenario it is wherever pytest ran. Unconditionally chdir'ing to the charm root, as the previous commit on this branch did, broke roughly eight charms in the curated cache. Switch to an audit-hook based deprecation path: by default leave the working directory alone but install a `sys.addaudithook` (scoped via `threading.local` to event dispatch) that emits a `DeprecationWarning` the first time charm code opens, lists, or scans a relative path that would resolve differently between the test cwd and the charm root. Set `SCENARIO_CHDIR_TO_CHARM_ROOT=1` to opt in to the Juju-matching behaviour now, ahead of the default flipping in a future major release. Co-Authored-By: Claude Opus 4.7 --- testing/src/scenario/_runtime.py | 100 +++++++++++++++++++++++++-- testing/tests/test_e2e/test_vroot.py | 63 ++++++++++++++++- 2 files changed, 155 insertions(+), 8 deletions(-) diff --git a/testing/src/scenario/_runtime.py b/testing/src/scenario/_runtime.py index 8e281096f..9bceecb37 100644 --- a/testing/src/scenario/_runtime.py +++ b/testing/src/scenario/_runtime.py @@ -8,8 +8,11 @@ import copy import dataclasses import os +import sys import tempfile +import threading import typing +import warnings from collections.abc import Iterator from contextlib import contextmanager from pathlib import Path @@ -40,6 +43,74 @@ RUNTIME_MODULE = Path(__file__).parent +_CWD_AUDIT_STATE = threading.local() + + +def _cwd_audit_hook(event: str, args: tuple[object, ...]) -> None: + """Audit hook that warns when charm code reads relative paths during an event. + + Under Juju, the working directory at hook start is the charm root. Scenario + historically left it as wherever pytest ran, so relative reads silently + resolve against the test directory. This hook flags any relative path + access that would resolve differently between the two, so charms can be + updated before the default flips. + """ + state = getattr(_CWD_AUDIT_STATE, 'current', None) + if state is None: + return + if event not in ('open', 'os.listdir', 'os.scandir'): + return + if state.get('reentrant'): + return + path = args[0] if args else None + if path is None or isinstance(path, int): + return + try: + path_str = os.fspath(path) + except TypeError: + return + if isinstance(path_str, bytes): + try: + path_str = path_str.decode() + except UnicodeDecodeError: + return + if not path_str or os.path.isabs(path_str): + return + if path_str in state['warned']: + return + state['reentrant'] = True + try: + old_path = os.path.normpath(os.path.join(state['test_cwd'], path_str)) + new_path = os.path.normpath(os.path.join(state['charm_root'], path_str)) + if old_path == new_path: + return + if not (os.path.exists(old_path) or os.path.exists(new_path)): + return + state['warned'].add(path_str) + warnings.warn( + f'Scenario charm code accessed relative path {path_str!r} during ' + f'event dispatch. Under Juju the working directory is the charm ' + f'root, but Scenario currently keeps the test runner working ' + f'directory; in a future major release Scenario will chdir to ' + f'the charm root to match Juju. Set ' + f'SCENARIO_CHDIR_TO_CHARM_ROOT=1 to opt in to the new behaviour ' + f'now, or use self.framework.charm_dir to make the lookup ' + f'explicit.', + DeprecationWarning, + stacklevel=3, + ) + finally: + state['reentrant'] = False + + +sys.addaudithook(_cwd_audit_hook) + + +def _chdir_to_charm_root_opt_in() -> bool: + value = os.getenv('SCENARIO_CHDIR_TO_CHARM_ROOT', '') + return value.lower() in ('true', '1', 'yes') or (value.isdigit() and int(value) != 0) + + class Runtime(Generic[CharmType]): """Charm runtime wrapper. @@ -322,11 +393,27 @@ def exec( os.environ.update(env) # In Juju, the current working directory is the charm root when a - # hook starts. Charms (and libraries) rely on this, e.g. to read - # files relative to it. Match that behaviour, restoring the - # original working directory once the event has been handled. + # hook starts. Charms (and libraries) rely on this, for example to + # read files relative to it. Scenario does not yet match that + # behaviour by default, to avoid breaking existing tests that + # depend on the test runner working directory. Set + # SCENARIO_CHDIR_TO_CHARM_ROOT=1 to opt in to the Juju behaviour; + # otherwise, an audit hook emits a DeprecationWarning the first + # time a relative path is accessed that would resolve differently + # under the new behaviour. previous_cwd = os.getcwd() - os.chdir(temporary_charm_root) + chdir_to_charm_root = _chdir_to_charm_root_opt_in() + if chdir_to_charm_root: + os.chdir(temporary_charm_root) + audit_state = None + else: + audit_state = { + 'test_cwd': previous_cwd, + 'charm_root': str(temporary_charm_root), + 'warned': set(), + 'reentrant': False, + } + _CWD_AUDIT_STATE.current = audit_state logger.info(' - entering ops.main (mocked)') from ._ops_main_mock import Ops @@ -371,7 +458,10 @@ def exec( if key not in previous_env: del os.environ[key] os.environ.update(previous_env) - os.chdir(previous_cwd) + if chdir_to_charm_root: + os.chdir(previous_cwd) + else: + _CWD_AUDIT_STATE.current = None logger.info(' - exited ops.main') logger.info('event dispatched. done.') diff --git a/testing/tests/test_e2e/test_vroot.py b/testing/tests/test_e2e/test_vroot.py index b48840f13..e812212cb 100644 --- a/testing/tests/test_e2e/test_vroot.py +++ b/testing/tests/test_e2e/test_vroot.py @@ -5,6 +5,7 @@ import os import tempfile +import warnings from collections.abc import Generator, Mapping from pathlib import Path from typing import Any @@ -64,21 +65,77 @@ class CwdCharm(CharmBase): def __init__(self, framework: Framework): super().__init__(framework) - # Charms may read files relative to the cwd, which Juju sets to the - # charm root. The cwd should match charm_dir during event handling. self.unit.status = ActiveStatus(os.getcwd()) -def test_cwd_is_charm_root_during_event(): +def test_cwd_unchanged_by_default(): cwd_before = os.getcwd() ctx = Context(CwdCharm, meta=dict(CwdCharm.META)) with ctx(ctx.on.start(), State()) as mgr: mgr.run() + # By default, Scenario leaves the test runner working directory in + # place rather than chdir'ing to the charm root. + assert mgr.charm.unit.status.message == cwd_before + assert os.getcwd() == cwd_before + + +def test_cwd_is_charm_root_when_opted_in(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('SCENARIO_CHDIR_TO_CHARM_ROOT', '1') + cwd_before = os.getcwd() + ctx = Context(CwdCharm, meta=dict(CwdCharm.META)) + with ctx(ctx.on.start(), State()) as mgr: + mgr.run() + # With the opt-in env var, cwd matches charm_dir to match Juju. assert mgr.charm.unit.status.message == str(mgr.charm.framework.charm_dir) # The original working directory is restored after the event is handled. assert os.getcwd() == cwd_before +class RelativeOpenCharm(CharmBase): + META: Mapping[str, Any] = {'name': 'my-charm'} + REL_PATH = 'metadata.yaml' + + def __init__(self, framework: Framework): + super().__init__(framework) + try: + with open(self.REL_PATH): + pass + except OSError: + pass + + +def test_relative_path_open_emits_deprecation_warning( + charm_virtual_root: Path, monkeypatch: pytest.MonkeyPatch +): + # Ensure the env var is not set so we exercise the default (warn) path. + monkeypatch.delenv('SCENARIO_CHDIR_TO_CHARM_ROOT', raising=False) + # The relative path the charm opens exists at charm_root but not at the + # test cwd, so behaviour would differ between current and future defaults. + (charm_virtual_root / RelativeOpenCharm.REL_PATH).write_text('name: my-charm\n') + ctx = Context( + RelativeOpenCharm, + meta=dict(RelativeOpenCharm.META), + charm_root=charm_virtual_root, + ) + with pytest.warns(DeprecationWarning, match='SCENARIO_CHDIR_TO_CHARM_ROOT'): + ctx.run(ctx.on.start(), State()) + + +def test_relative_path_open_no_warning_when_opted_in( + charm_virtual_root: Path, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setenv('SCENARIO_CHDIR_TO_CHARM_ROOT', '1') + (charm_virtual_root / RelativeOpenCharm.REL_PATH).write_text('name: my-charm\n') + ctx = Context( + RelativeOpenCharm, + meta=dict(RelativeOpenCharm.META), + charm_root=charm_virtual_root, + ) + with warnings.catch_warnings(): + warnings.simplefilter('error', DeprecationWarning) + ctx.run(ctx.on.start(), State()) + + def test_charm_virtual_root_cleanup_if_exists(charm_virtual_root: Path): meta_file = charm_virtual_root / 'metadata.yaml' raw_ori_meta = yaml.safe_dump({'name': 'karl'})