From a51149a2e1d382c05ab0f607fe7420c170a5b96e Mon Sep 17 00:00:00 2001 From: CleverThis Date: Thu, 11 Jun 2026 12:53:53 -0400 Subject: [PATCH] test(merge): scope ca_merge_ leftover check to scenario-local tmpdirs The "no temporary merge files should remain on disk" step in features/sandbox_merge_strategies.feature:36 scanned global tempfile.gettempdir() for any ca_merge_* entry and asserted zero. Under behave-parallel --processes 8 a sibling scenario's in-flight tmpdir in the same shared /tmp could be observed mid-merge and trip the assertion, even though GitMergeStrategy.merge cleans up its own tmpdir in a finally block (src/cleveragents/infrastructure/sandbox/merge.py:151-155). Patch tempfile.mkdtemp inside cleveragents.infrastructure.sandbox.merge for the @when step to track the tmpdirs this scenario's merge actually creates, then assert those specific paths are gone in the @then step. Production merge code is unchanged; the contract under test (cleanup-on-success) is preserved; sibling scenarios sharing /tmp no longer race the assertion. ISSUES CLOSED: #7527 --- .../steps/sandbox_merge_strategies_steps.py | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/features/steps/sandbox_merge_strategies_steps.py b/features/steps/sandbox_merge_strategies_steps.py index a15b68894..68540cf91 100644 --- a/features/steps/sandbox_merge_strategies_steps.py +++ b/features/steps/sandbox_merge_strategies_steps.py @@ -99,7 +99,22 @@ def step_merge_git(context: Context) -> None: """Perform a merge using the GitMergeStrategy.""" _assert_git_available() strategy = GitMergeStrategy() - context.merge_result = strategy.merge(context.base, context.ours, context.theirs) + created_tmpdirs: list[str] = [] + real_mkdtemp = tempfile.mkdtemp + + def _tracked_mkdtemp(*args, **kwargs): + path = real_mkdtemp(*args, **kwargs) + created_tmpdirs.append(path) + return path + + with patch( + "cleveragents.infrastructure.sandbox.merge.tempfile.mkdtemp", + side_effect=_tracked_mkdtemp, + ): + context.merge_result = strategy.merge( + context.base, context.ours, context.theirs + ) + context.ca_merge_tmpdirs = created_tmpdirs @when("the changes are merged using the git strategy with git unavailable") @@ -184,10 +199,20 @@ def step_merged_empty(context: Context) -> None: @then("no temporary merge files should remain on disk") def step_no_temp_files(context: Context) -> None: - """Verify that no ca_merge_ temp directories remain.""" - tmp_root = tempfile.gettempdir() - leftover = [d for d in os.listdir(tmp_root) if d.startswith("ca_merge_")] - assert len(leftover) == 0, f"Found leftover temp directories: {leftover}" + """Verify the tmpdirs THIS scenario's merge created have been cleaned up. + + Checking the specific paths the merge created (rather than scanning the + shared temp directory for any ``ca_merge_*`` entry) keeps the assertion + robust under ``behave-parallel`` — a sibling scenario's in-flight tmpdir + in the same shared ``/tmp`` is not our concern. + """ + created = getattr(context, "ca_merge_tmpdirs", []) + assert created, ( + "Expected the merge step to record at least one tracked tmpdir; " + "got none." + ) + leftover = [d for d in created if os.path.exists(d)] + assert not leftover, f"Found leftover temp directories: {leftover}" @then("the merged content should be the incoming side")