From 6f3a10c04ee383cc98696cd3453dfbc51f5ce340 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Tue, 5 May 2026 00:48:26 +0000 Subject: [PATCH] fix(cleanup): isolate cache invalidation tests with private temp directory Introduce _IsolatedCleanupService subclass that overrides _get_sandbox_dirs() to scan a per-scenario private temp directory instead of the system-wide tempfile.gettempdir(). This prevents concurrent CI test workers from interfering with each other by reading or deleting sandbox directories created by other scenarios running in parallel. All five BDD scenarios now use the isolated service and create sandbox directories exclusively inside a private mkdtemp() directory that is cleaned up in after_scenario via context._cleanup_handlers. ISSUES CLOSED: #7527 --- ...leanup_service_cache_invalidation_steps.py | 133 +++++++++++++----- 1 file changed, 98 insertions(+), 35 deletions(-) diff --git a/features/steps/cleanup_service_cache_invalidation_steps.py b/features/steps/cleanup_service_cache_invalidation_steps.py index 8ae99dbc8..61d904298 100644 --- a/features/steps/cleanup_service_cache_invalidation_steps.py +++ b/features/steps/cleanup_service_cache_invalidation_steps.py @@ -5,6 +5,14 @@ so that a subsequent ``scan()`` call re-reads the filesystem instead of returning already-deleted paths. All tests use real filesystem operations — no mocks, no patches. + +Test isolation strategy +----------------------- +Each scenario creates a private temporary directory (via ``tempfile.mkdtemp()``) +and uses an ``_IsolatedCleanupService`` subclass that overrides +``_get_sandbox_dirs()`` to scan only that private directory. This prevents +the test from interfering with other concurrently-running scenarios that also +create ``ca-sandbox-*`` directories in the shared system temp directory. """ from __future__ import annotations @@ -36,6 +44,45 @@ _STALE_MTIME_OFFSET = 999_999 _CLEANUP_MAX_AGE_ENV = "CLEVERAGENTS_CLEANUP_SANDBOX_MAX_AGE_HOURS" +class _IsolatedCleanupService(CleanupService): + """CleanupService subclass that scans a private temp directory. + + Overrides ``_get_sandbox_dirs()`` to look only in ``_private_tmp`` + instead of the system-wide ``tempfile.gettempdir()``. This prevents + the test from reading or deleting sandbox directories created by other + concurrently-running scenarios. + """ + + def __init__(self, settings: Settings, private_tmp: Path) -> None: + super().__init__(settings) + self._private_tmp = private_tmp + + def _get_sandbox_dirs(self) -> list[Path]: + """Return sandbox dirs from the private temp directory only.""" + if self._sandbox_dirs_cache is not None: + return self._sandbox_dirs_cache + if not self._private_tmp.exists(): + self._sandbox_dirs_cache = [] + return self._sandbox_dirs_cache + dirs: list[Path] = [] + try: + entries = list(self._private_tmp.iterdir()) + except OSError: + self._sandbox_dirs_cache = [] + return self._sandbox_dirs_cache + for p in entries: + try: + is_dir = p.is_dir() + except OSError: + continue + if is_dir and any( + p.name.startswith(pfx) for pfx in ("ca-sandbox-", "ca-cow-sandbox-") + ): + dirs.append(p) + self._sandbox_dirs_cache = dirs + return dirs + + def _make_settings(**overrides: object) -> Settings: """Create a real Settings instance with optional field overrides. @@ -66,28 +113,40 @@ def _unique_sandbox_name(prefix: str = "ca-sandbox-test") -> str: return f"{prefix}-{uuid.uuid4().hex[:12]}" -def _make_real_stale_sandbox(name: str) -> Path: - """Create a real stale sandbox directory in the system temp dir. +def _make_private_tmp(context: Context) -> Path: + """Create a private temp directory for this scenario and register cleanup.""" + private_tmp = Path(tempfile.mkdtemp(prefix="ca-test-isolation-")) + + def _remove() -> None: + if private_tmp.exists(): + shutil.rmtree(str(private_tmp), ignore_errors=True) + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_remove) + return private_tmp + + +def _make_real_stale_sandbox(parent: Path, name: str) -> Path: + """Create a real stale sandbox directory inside *parent*. Sets the mtime far in the past so ``_is_sandbox_stale`` returns True when ``cleanup_sandbox_max_age_hours=1``. """ - tmp = Path(tempfile.gettempdir()) - sandbox = tmp / name + sandbox = parent / name sandbox.mkdir(exist_ok=True) old_time = time.time() - _STALE_MTIME_OFFSET os.utime(str(sandbox), (old_time, old_time)) return sandbox -def _make_real_fresh_sandbox(name: str) -> Path: - """Create a real fresh (non-stale) sandbox directory in the system temp dir. +def _make_real_fresh_sandbox(parent: Path, name: str) -> Path: + """Create a real fresh (non-stale) sandbox directory inside *parent*. Uses the current mtime so the directory is not considered stale under the default ``cleanup_sandbox_max_age_hours=48`` setting. """ - tmp = Path(tempfile.gettempdir()) - sandbox = tmp / name + sandbox = parent / name sandbox.mkdir(exist_ok=True) return sandbox @@ -109,21 +168,20 @@ def _register_cleanup(context: Context, path: Path) -> None: @given("cache invalidation has a CleanupService with a pre-populated sandbox cache") def step_cache_inv_service_prepopulated(context: Context) -> None: - """Create a CleanupService whose cache holds real stale sandbox paths.""" + """Create an isolated CleanupService whose cache holds real stale sandbox paths.""" + private_tmp = _make_private_tmp(context) context.cache_inv_settings = _make_settings( cleanup_sandbox_max_age_hours=_STALE_MAX_AGE_HOURS ) - context.cache_inv_service = CleanupService(context.cache_inv_settings) + context.cache_inv_service = _IsolatedCleanupService( + context.cache_inv_settings, private_tmp + ) - # Create two real stale sandbox directories in the system temp dir + # Create two real stale sandbox directories in the private temp dir name_a = _unique_sandbox_name("ca-sandbox-plan-aaa") name_b = _unique_sandbox_name("ca-sandbox-plan-bbb") - dir_a = _make_real_stale_sandbox(name_a) - dir_b = _make_real_stale_sandbox(name_b) - - # Register cleanup in case purge does not delete them - _register_cleanup(context, dir_a) - _register_cleanup(context, dir_b) + dir_a = _make_real_stale_sandbox(private_tmp, name_a) + dir_b = _make_real_stale_sandbox(private_tmp, name_b) # Pre-populate the cache with the real Path objects context.cache_inv_service._sandbox_dirs_cache = [dir_a, dir_b] @@ -132,36 +190,37 @@ def step_cache_inv_service_prepopulated(context: Context) -> None: @given("cache invalidation has a CleanupService with stale sandbox directories on disk") def step_cache_inv_service_real_stale_dirs(context: Context) -> None: - """Create real stale sandbox directories in the system temp dir.""" + """Create real stale sandbox directories in a private temp dir.""" + private_tmp = _make_private_tmp(context) name_a = _unique_sandbox_name("ca-sandbox-planA") name_b = _unique_sandbox_name("ca-sandbox-planB") - dir_a = _make_real_stale_sandbox(name_a) - dir_b = _make_real_stale_sandbox(name_b) - - # Register cleanup in case purge does not delete them - _register_cleanup(context, dir_a) - _register_cleanup(context, dir_b) + dir_a = _make_real_stale_sandbox(private_tmp, name_a) + dir_b = _make_real_stale_sandbox(private_tmp, name_b) # Use minimum allowed max_age_hours=1; dirs are ~11.5 days old so they # are immediately stale. context.cache_inv_settings = _make_settings( cleanup_sandbox_max_age_hours=_STALE_MAX_AGE_HOURS ) - context.cache_inv_service = CleanupService(context.cache_inv_settings) + context.cache_inv_service = _IsolatedCleanupService( + context.cache_inv_settings, private_tmp + ) context.cache_inv_real_dirs = [dir_a, dir_b] @given("cache invalidation has a CleanupService with a fresh non-stale sandbox cache") def step_cache_inv_service_nonstale_cache(context: Context) -> None: - """Create a CleanupService whose cache holds a real non-stale sandbox dir.""" + """Create an isolated CleanupService whose cache holds a real non-stale sandbox dir.""" + private_tmp = _make_private_tmp(context) # Use default max_age_hours=48 so the fresh dir (current mtime) is NOT stale context.cache_inv_settings = _make_settings() - context.cache_inv_service = CleanupService(context.cache_inv_settings) + context.cache_inv_service = _IsolatedCleanupService( + context.cache_inv_settings, private_tmp + ) - # Create a real fresh (non-stale) sandbox directory + # Create a real fresh (non-stale) sandbox directory in the private temp dir name = _unique_sandbox_name("ca-sandbox-plan-fresh") - fresh_dir = _make_real_fresh_sandbox(name) - _register_cleanup(context, fresh_dir) + fresh_dir = _make_real_fresh_sandbox(private_tmp, name) # Pre-populate the cache with the real Path object context.cache_inv_service._sandbox_dirs_cache = [fresh_dir] @@ -189,11 +248,13 @@ def step_cache_inv_scan_purge_scan(context: Context) -> None: svc._sandbox_dirs_cache = list(real_dirs) context.cache_inv_first_scan = svc.scan() - # Purge: actually deletes the directories and invalidates the cache + # Purge: actually deletes the directories and invalidates the cache. + # Because svc is an _IsolatedCleanupService, _get_sandbox_dirs() will + # re-read only the private temp dir — not the system-wide /tmp. context.cache_inv_purge_report = svc.purge() # Second scan: cache was invalidated, so _get_sandbox_dirs re-reads - # the filesystem. The real dirs are now gone from disk. + # the private temp dir. The real dirs are now gone from disk. context.cache_inv_second_scan = svc.scan() @@ -214,13 +275,15 @@ def step_cache_inv_purge_then_get_dirs(context: Context) -> None: report = CleanupReport(dry_run=False) svc._purge_sandboxes(report) - # Create a new real sandbox dir so _get_sandbox_dirs has something to find + # Create a new real sandbox dir in the private temp dir so + # _get_sandbox_dirs has something to find after cache invalidation. + private_tmp = svc._private_tmp new_name = _unique_sandbox_name("ca-sandbox-plan-new") - new_dir = _make_real_stale_sandbox(new_name) + new_dir = _make_real_stale_sandbox(private_tmp, new_name) _register_cleanup(context, new_dir) context.cache_inv_new_dir = new_dir - # Call _get_sandbox_dirs to repopulate the cache from the real filesystem + # Call _get_sandbox_dirs to repopulate the cache from the private temp dir context.cache_inv_repopulated = svc._get_sandbox_dirs()