From 2fafcaf5ab5297b0cc2f19ae19a3cb3f74663b77 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Thu, 30 Jul 2026 07:58:43 -0700 Subject: [PATCH] fix: serialize testing cwd invocations --- README.md | 13 +++++--- lib/python/base_cli/testing.py | 23 +++++++++----- tests/test_testing.py | 56 ++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 4ab3f5a..6a8805d 100644 --- a/README.md +++ b/README.md @@ -481,11 +481,14 @@ def test_command(tmp_path: Path) -> None: assert "hello Ada" in result.stdout ``` -The helper wraps Click's `CliRunner`, sets `HOME` when requested, supplies -`cwd` to Base's context discovery without mutating process-global cwd, and keeps -stderr separate on Click versions that support it. Use `cwd` for commands whose -behavior depends on project discovery, including tests that intentionally run -outside a Base project. Pass +The helper wraps Click's `CliRunner`, sets `HOME` when requested, and supplies +`cwd` to Base's context discovery by temporarily changing process-global cwd +for the duration of the invocation. Calls that use `cwd` are serialized and +the caller's cwd is restored afterward, but this remains process-global: do not +use it concurrently with code that changes cwd outside `invoke()` or from +threads spawned by the invoked command. Use `cwd` for commands whose behavior +depends on project discovery, including tests that intentionally run outside a +Base project. Pass `manifest={...}` with `cwd` to write a temporary `base_manifest.yaml` before the command runs. diff --git a/lib/python/base_cli/testing.py b/lib/python/base_cli/testing.py index 693fdee..38e057a 100644 --- a/lib/python/base_cli/testing.py +++ b/lib/python/base_cli/testing.py @@ -4,6 +4,7 @@ import os from collections.abc import Mapping from pathlib import Path +from threading import RLock from typing import TYPE_CHECKING, Any from .paths import use_working_dir @@ -13,6 +14,9 @@ from click.testing import Result +_INVOKE_CWD_LOCK = RLock() + + # pylint: disable=too-many-arguments def invoke( app: Any, @@ -42,15 +46,18 @@ def invoke( if "mix_stderr" in inspect.signature(CliRunner).parameters: runner_kwargs["mix_stderr"] = False runner = CliRunner(**runner_kwargs) - with use_working_dir(cwd_path): - if cwd_path is None: + if cwd_path is None: + with use_working_dir(None): return runner.invoke(app.click_command, args or [], env=invoke_env) - original_cwd = Path.cwd() - os.chdir(cwd_path) - try: - return runner.invoke(app.click_command, args or [], env=invoke_env) - finally: - os.chdir(original_cwd) + + with _INVOKE_CWD_LOCK: + with use_working_dir(cwd_path): + original_cwd = Path.cwd() + os.chdir(cwd_path) + try: + return runner.invoke(app.click_command, args or [], env=invoke_env) + finally: + os.chdir(original_cwd) def _write_manifest_fixture(cwd: Path, manifest: Mapping[str, Any]) -> None: diff --git a/tests/test_testing.py b/tests/test_testing.py index b5f7a7a..fe8404d 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -6,8 +6,11 @@ import subprocess import sys import tempfile +import threading import unittest +from concurrent.futures import ThreadPoolExecutor from pathlib import Path +from unittest import mock import base_cli from base_cli.testing import invoke @@ -133,3 +136,56 @@ def main(ctx: base_cli.Context) -> None: self.assertEqual(result.exit_code, 0, result.output) self.assertIsNone(seen["project_root"]) self.assertIsNone(seen["manifest_path"]) + + def test_invoke_with_cwd_serializes_process_cwd_mutation(self) -> None: + app = base_cli.App(name="testing-cwd-serialization", log_to_file=False) + + @app.command() + def main() -> None: + return None + + first_started = threading.Event() + second_started = threading.Event() + release_first = threading.Event() + state_lock = threading.Lock() + observed_cwds: list[Path] = [] + active_calls = 0 + max_active_calls = 0 + + def fake_invoke(_runner: object, *_args: object, **_kwargs: object) -> object: + nonlocal active_calls, max_active_calls + with state_lock: + active_calls += 1 + max_active_calls = max(max_active_calls, active_calls) + observed_cwds.append(Path.cwd()) + if len(observed_cwds) == 1: + first_started.set() + else: + second_started.set() + if not release_first.wait(timeout=5): + raise AssertionError("timed out waiting to release invoke") + with state_lock: + active_calls -= 1 + return mock.sentinel.result + + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + first_cwd = root / "first" + second_cwd = root / "second" + first_cwd.mkdir() + second_cwd.mkdir() + original_cwd = Path.cwd() + + with mock.patch("click.testing.CliRunner.invoke", new=fake_invoke): + with ThreadPoolExecutor(max_workers=2) as executor: + first = executor.submit(invoke, app, home=root / "home", cwd=first_cwd) + self.assertTrue(first_started.wait(timeout=5)) + second = executor.submit(invoke, app, home=root / "home", cwd=second_cwd) + self.assertFalse(second_started.wait(timeout=0.1)) + release_first.set() + self.assertIs(first.result(timeout=5), mock.sentinel.result) + self.assertIs(second.result(timeout=5), mock.sentinel.result) + + self.assertEqual(max_active_calls, 1) + self.assertEqual(set(observed_cwds), {first_cwd.resolve(), second_cwd.resolve()}) + self.assertEqual(Path.cwd(), original_cwd)