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
This commit is contained in:
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user