On Fri, Aug 1, 2025 at 1:31 AM Patrick Steinhardt <ps@xxxxxx> wrote: > > On Tue, Jul 22, 2025 at 03:23:08PM +0000, Elijah Newren via GitGitGadget wrote: > > diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh > > index f48ed6d03534..69de7a3b84af 100755 > > --- a/t/t6423-merge-rename-directories.sh > > +++ b/t/t6423-merge-rename-directories.sh > > @@ -5092,7 +5111,85 @@ test_expect_success '12n: Directory rename transitively makes rename back to sel > > git checkout -q B^0 && > > > > test_must_fail git cherry-pick A^0 >out && > > - grep "CONFLICT (file location).*should perhaps be moved" out > > + grep "CONFLICT (file location).*should perhaps be moved" out && > > Let's use `test_grep` while at it. Oh, right, it was test_i18ngrep that we weren't supposed to use. Yeah, I can switch over to test_grep. > [snip] > > +test_expect_failure '12n2: Directory rename transitively makes rename back to self' ' > > + test_setup_12n2 && > > + ( > > + cd 12n2 && > > + > > + git checkout -q B^0 && > > + > > + # NOTE: Since merge.directoryRenames=true, there is no path > > + # conflict for world vs. tools/world; it should end up at > > + # world. The fact that world was unmodified on side A, means > > + # there was no content conflict; we should just take the > > + # content from side B -- i.e. delete the file. So merging > > + # could just delete world. > > + # > > + # However, rename-to-self-via-directory-rename is a bit more > > + # challenging. Relax this test to allow world to be treated > > + # as a modify/delete conflict as well. > > + > > + test_might_fail git -c merge.directoryRenames=true merge A^0 >out && > > + > > + # Should have 1 entry for hello, and either 0 or 2 for world > > + test_stdout_line_count = 1 git ls-files -s hello && > > + test_stdout_line_count != 1 git ls-files -s world && > > + if test_stdout_line_count != 0 git ls-files -s world > > + then > > + grep "CONFLICT (modify/delete).*world deleted in HEAD" out > > Here, as well. Will do. > > + fi > > ) > > ' > > I found it to be a bit weird that we have this conditional here. > Shouldn't we expect one particular outcome? Even if multiple outcomes > would be techincally correct I think we should expect one particular > result, but we may add a comment to explain that different output would > be fine, too. Isn't that exactly what I did, with the note I'll copy below? > > + # NOTE: Since merge.directoryRenames=true, there is no path > > + # conflict for world vs. tools/world; it should end up at > > + # world. The fact that world was unmodified on side A, means > > + # there was no content conflict; we should just take the > > + # content from side B -- i.e. delete the file. So merging > > + # could just delete world. > > + # > > + # However, rename-to-self-via-directory-rename is a bit more > > + # challenging. Relax this test to allow world to be treated > > + # as a modify/delete conflict as well.
