[PATCH b4 6/6] review-tui: test take->merge conflict resolution
Christian Brauner <[email protected]> Tue, 23 Jun 2026 18:01:34 +0200
| Newsgroups | org.kernel.linux.tools |
|---|---|
| Message-ID | <20260623-20260623-review-tui-take-am-worktree-v1-6-e8a2140a287e@kernel.org> |
Cover the new in-place merge conflict resolution: the shared _resolve_worktree_take_conflict() helper (resolve, abort, and an unfinished throwaway worktree being kept) and an end-to-end _do_take_merge() run through a real conflicted merge. The key invariant is that finishing the conflicted merge in place keeps the merged-in side fully reachable -- unlike "b4 shazam --resolve", which replays not-yet-applied patches with `git apply --3way` + `git add -u` and so silently drops a patch that only adds a new file. The tests assert a real two-parent merge commit results, the merged side is an ancestor of HEAD, and a brand-new file added on that side survives. Signed-off-by: Christian Brauner (Amutable) <[email protected]> --- src/tests/test_tui_tracking.py | 236 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 235 insertions(+), 1 deletion(-) diff --git a/src/tests/test_tui_tracking.py b/src/tests/test_tui_tracking.py index f21b98d..8aa0ad3 100644 --- a/src/tests/test_tui_tracking.py +++ b/src/tests/test_tui_tracking.py @@ -15,7 +15,7 @@ import datetime import email.message import os import pathlib -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from unittest.mock import patch import pytest @@ -38,9 +38,12 @@ from b4.review_tui._modals import ( from b4.review_tui._tracking_app import ( TrackedSeriesItem, TrackingApp, + _resolve_worktree_take_conflict, _shazam_merge_flags, _take_worktree, + _TakeWorktree, _worktree_for_branch, + _worktree_merge_in_progress, ) # --------------------------------------------------------------------------- @@ -3773,3 +3776,234 @@ class TestAmTakeWorktree: gitdir, ['status', '--porcelain', '--untracked-files=no'] ) assert ecode == 0 and status.strip() == '' + + +# --------------------------------------------------------------------------- +# take->merge conflict resolution (must never drop a non-empty commit) +# --------------------------------------------------------------------------- + + +def _conflict_base(gitdir: str) -> str: + """Commit a shared file onto the current branch; return the new HEAD sha.""" + (pathlib.Path(gitdir) / 'shared.txt').write_text('a\nb\nc\n') + b4.git_run_command(gitdir, ['add', 'shared.txt']) + ecode, _ = b4.git_run_command(gitdir, ['commit', '-m', 'add shared.txt']) + assert ecode == 0 + ecode, base = b4.git_run_command(gitdir, ['rev-parse', 'HEAD']) + assert ecode == 0 + return base.strip() + + +def _conflicted_merge_worktree(gitdir: str, tmp_path: pathlib.Path) -> Tuple[str, str]: + """Park a worktree on a conflicted ``git merge``. + + The merged-in ("theirs") side edits the shared file (forcing the conflict) + *and* adds a brand-new file -- the kind of non-empty change that + ``b4 shazam --resolve`` silently drops via ``git add -u``. Returns + (worktree_path, pre_merge_head). + """ + base = _conflict_base(gitdir) + b4.git_run_command(gitdir, ['branch', 'ours', base]) + b4.git_run_command(gitdir, ['branch', 'theirs', base]) + + ours_wt = str(tmp_path / 'ours-wt') + ecode, _ = b4.git_run_command(gitdir, ['worktree', 'add', ours_wt, 'ours']) + assert ecode == 0 + (pathlib.Path(ours_wt) / 'shared.txt').write_text('a\nOURS\nc\n') + b4.git_run_command(ours_wt, ['add', 'shared.txt']) + ecode, _ = b4.git_run_command(ours_wt, ['commit', '-m', 'ours edits shared']) + assert ecode == 0 + + theirs_wt = str(tmp_path / 'theirs-wt') + ecode, _ = b4.git_run_command(gitdir, ['worktree', 'add', theirs_wt, 'theirs']) + assert ecode == 0 + (pathlib.Path(theirs_wt) / 'shared.txt').write_text('a\nTHEIRS\nc\n') + (pathlib.Path(theirs_wt) / 'newfile.txt').write_text('brand new\n') + b4.git_run_command(theirs_wt, ['add', '-A']) + ecode, _ = b4.git_run_command( + theirs_wt, ['commit', '-m', 'theirs edits shared and adds newfile'] + ) + assert ecode == 0 + b4.git_run_command(gitdir, ['worktree', 'remove', '--force', theirs_wt]) + + ecode, pre = b4.git_run_command(ours_wt, ['rev-parse', 'HEAD']) + assert ecode == 0 + pre = pre.strip() + ecode, _ = b4.git_run_command( + ours_wt, ['merge', '--no-ff', 'theirs'], logstderr=True + ) + assert ecode != 0 # conflict on shared.txt + return ours_wt, pre + + +def _resolve_shared(*_args: Any, **kwargs: Any) -> None: + """_suspend_to_shell stand-in: resolve the conflict and finish the merge.""" + cwd = kwargs['cwd'] + (pathlib.Path(cwd) / 'shared.txt').write_text('a\nRESOLVED\nc\n') + b4.git_run_command(cwd, ['add', 'shared.txt']) + ecode, _ = b4.git_run_command(cwd, ['commit', '--no-edit']) + assert ecode == 0 + + +class TestMergeConflictResolution: + """take->merge finishes the *same* real merge, dropping nothing. + + Unlike ``b4 shazam --resolve`` (which replays not-yet-applied patches with + ``git apply --3way`` + ``git add -u`` and so silently omits a patch that + adds a new file), the conflicted ``git merge`` is completed in place, so the + merged-in side stays fully reachable and every added file survives. + """ + + def test_in_progress_detects_and_clears( + self, gitdir: str, tmp_path: pathlib.Path + ) -> None: + wt, _pre = _conflicted_merge_worktree(gitdir, tmp_path) + assert _worktree_merge_in_progress(wt) is True + b4.git_run_command(wt, ['merge', '--abort']) + assert _worktree_merge_in_progress(wt) is False + + def test_resolution_keeps_added_file( + self, gitdir: str, tmp_path: pathlib.Path + ) -> None: + wt, pre = _conflicted_merge_worktree(gitdir, tmp_path) + handle = _TakeWorktree(wt, is_temp=False) + with patch( + 'b4.review_tui._tracking_app._suspend_to_shell', + side_effect=_resolve_shared, + ): + ok = _resolve_worktree_take_conflict( + handle, 'merge', pre, _worktree_merge_in_progress + ) + assert ok is True + # A real two-parent merge commit... + ecode, parents = b4.git_run_command(wt, ['rev-list', '--parents', '-1', 'HEAD']) + assert ecode == 0 and len(parents.split()) == 3 + # ...with the merged-in side fully reachable (no commit dropped)... + ecode, theirs = b4.git_run_command(wt, ['rev-parse', 'theirs']) + assert ecode == 0 + ecode, _ = b4.git_run_command( + wt, ['merge-base', '--is-ancestor', theirs.strip(), 'HEAD'] + ) + assert ecode == 0 + # ...and the brand-new file (shazam --resolve drops it) is present. + ecode, content = b4.git_run_command(wt, ['show', 'HEAD:newfile.txt']) + assert ecode == 0 and content.strip() == 'brand new' + ecode, shared = b4.git_run_command(wt, ['show', 'HEAD:shared.txt']) + assert ecode == 0 and 'RESOLVED' in shared + + def test_abort_returns_false_and_restores_head( + self, gitdir: str, tmp_path: pathlib.Path + ) -> None: + wt, pre = _conflicted_merge_worktree(gitdir, tmp_path) + handle = _TakeWorktree(wt, is_temp=False) + + def _abort(*_a: Any, **kw: Any) -> None: + b4.git_run_command(kw['cwd'], ['merge', '--abort']) + + with patch('b4.review_tui._tracking_app._suspend_to_shell', side_effect=_abort): + ok = _resolve_worktree_take_conflict( + handle, 'merge', pre, _worktree_merge_in_progress + ) + assert ok is False + ecode, head = b4.git_run_command(wt, ['rev-parse', 'HEAD']) + assert ecode == 0 and head.strip() == pre + assert _worktree_merge_in_progress(wt) is False + + def test_incomplete_keeps_throwaway( + self, gitdir: str, tmp_path: pathlib.Path + ) -> None: + wt, pre = _conflicted_merge_worktree(gitdir, tmp_path) + handle = _TakeWorktree(wt, is_temp=True) + # User exits the shell without finishing the merge. + with patch('b4.review_tui._tracking_app._suspend_to_shell'): + ok = _resolve_worktree_take_conflict( + handle, 'merge', pre, _worktree_merge_in_progress + ) + assert ok is False + assert handle._keep is True + assert _worktree_merge_in_progress(wt) is True + + +def _series_mbox_add_and_edit(gitdir: str, tmp_path: pathlib.Path, base: str) -> bytes: + """Two patches at *base*: add newfile.txt, then edit shared.txt line 2.""" + wt = str(tmp_path / 'seriesgen') + ecode, _ = b4.git_run_command(gitdir, ['worktree', 'add', '--detach', wt, base]) + assert ecode == 0 + try: + (pathlib.Path(wt) / 'newfile.txt').write_text('brand new\n') + b4.git_run_command(wt, ['add', 'newfile.txt']) + ecode, _ = b4.git_run_command(wt, ['commit', '-m', 'add newfile.txt']) + assert ecode == 0 + (pathlib.Path(wt) / 'shared.txt').write_text('a\nPATCH\nc\n') + b4.git_run_command(wt, ['add', 'shared.txt']) + ecode, _ = b4.git_run_command(wt, ['commit', '-m', 'edit shared.txt']) + assert ecode == 0 + ecode, patches = b4.git_run_command( + wt, ['format-patch', '--stdout', f'{base}..HEAD'] + ) + assert ecode == 0 + finally: + b4.git_run_command(gitdir, ['worktree', 'remove', '--force', wt]) + return patches.encode() + + +class TestDoTakeMergeConflict: + """End-to-end: _do_take_merge resolves a real merge conflict in place.""" + + def _run(self, gitdir: str, target_branch: str, ambytes: bytes) -> None: + from types import SimpleNamespace + + change_id = 'merge-conflict-e2e' + review_branch = _create_review_branch(gitdir, change_id, status='reviewing') + app = TrackingApp.__new__(TrackingApp) + app._identifier = None # type: ignore[attr-defined] + app._selected_series = {} # type: ignore[attr-defined] + app._prepare_am_messages = lambda *a, **k: ambytes # type: ignore[method-assign] + take_screen: Any = SimpleNamespace( + target_result=target_branch, + add_signoff=False, + add_link=False, + accept_series=False, + ) + with ( + patch('b4.review_tui._tracking_app._wait_for_enter'), + patch( + 'b4.review_tui._tracking_app._suspend_to_shell', + side_effect=_resolve_shared, + ), + ): + app._do_take_merge( + change_id, review_branch, take_screen, {'subject': 'x'}, None + ) + + def test_resolves_without_dropping_added_file( + self, gitdir: str, tmp_path: pathlib.Path + ) -> None: + base = _conflict_base(gitdir) + # Target diverges on shared.txt and is checked out nowhere (throwaway). + b4.git_run_command(gitdir, ['branch', 'target', base]) + twt = str(tmp_path / 'target-edit') + ecode, _ = b4.git_run_command(gitdir, ['worktree', 'add', twt, 'target']) + assert ecode == 0 + (pathlib.Path(twt) / 'shared.txt').write_text('a\nTARGET\nc\n') + b4.git_run_command(twt, ['add', 'shared.txt']) + b4.git_run_command(twt, ['commit', '-m', 'target diverges']) + b4.git_run_command(gitdir, ['worktree', 'remove', '--force', twt]) + + ambytes = _series_mbox_add_and_edit(gitdir, tmp_path, base) + self._run(gitdir, 'target', ambytes) + + # target advanced to a real two-parent merge commit... + ecode, parents = b4.git_run_command( + gitdir, ['rev-list', '--parents', '-1', 'target'] + ) + assert ecode == 0 and len(parents.split()) == 3 + # ...the added-file patch survived (the shazam --resolve drop bug)... + ecode, content = b4.git_run_command(gitdir, ['show', 'target:newfile.txt']) + assert ecode == 0 and content.strip() == 'brand new' + ecode, shared = b4.git_run_command(gitdir, ['show', 'target:shared.txt']) + assert ecode == 0 and 'RESOLVED' in shared + # ...and the throwaway worktree was cleaned up. + common_dir = b4.git_get_common_dir(gitdir) + assert common_dir is not None + assert not os.path.isdir(os.path.join(common_dir, 'b4-take-worktree')) -- 2.53.0