Re: [PATCH v2] merge-ort: avoid assuming all renames detected

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 30, 2022 at 2:54 AM SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
>
> On Mon, Jan 17, 2022 at 06:25:55PM +0000, Elijah Newren via GitGitGadget wrote:
> > diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
> > index 035edc40b1e..f2bc8a7d2a2 100755
> > --- a/t/t6429-merge-sequence-rename-caching.sh
> > +++ b/t/t6429-merge-sequence-rename-caching.sh
> > @@ -697,4 +697,71 @@ test_expect_success 'caching renames only on upstream side, part 2' '
> >       )
> >  '
> >
> > +#
> > +# The following testcase just creates two simple renames (slightly modified
> > +# on both sides but without conflicting changes), and a directory full of
> > +# files that are otherwise uninteresting.  The setup is as follows:
> > +#
> > +#   base:     unrelated/<BUNCH OF FILES>
> > +#             numbers
> > +#             values
> > +#   upstream: modify: numbers
> > +#             modify: values
> > +#   topic:    add: unrelated/foo
> > +#             modify: numbers
> > +#             modify: values
> > +#             rename: numbers -> sequence
> > +#             rename: values -> progression
> > +#
> > +# This is a trivial rename case, but we're curious what happens with a very
> > +# low renameLimit interacting with the restart optimization trying to notice
> > +# that unrelated/ looks like a trivial merge candidate.
> > +#
> > +test_expect_success 'avoid assuming we detected renames' '
> > +     git init redo-weirdness &&
> > +     (
> > +             cd redo-weirdness &&
> > +
> > +             mkdir unrelated &&
> > +             for i in $(test_seq 1 10)
> > +             do
> > +                     >unrelated/$i
> > +             done &&
> > +             test_seq  2 10 >numbers &&
> > +             test_seq 12 20 >values &&
> > +             git add numbers values unrelated/ &&
> > +             git commit -m orig &&
> > +
> > +             git branch upstream &&
> > +             git branch topic &&
> > +
> > +             git switch upstream &&
> > +             test_seq  1 10 >numbers &&
> > +             test_seq 11 20 >values &&
> > +             git add numbers &&
> > +             git commit -m "Some tweaks" &&
> > +
> > +             git switch topic &&
> > +
> > +             >unrelated/foo &&
> > +             test_seq  2 12 >numbers &&
> > +             test_seq 12 22 >values &&
> > +             git add numbers values unrelated/ &&
> > +             git mv numbers sequence &&
> > +             git mv values progression &&
> > +             git commit -m A &&
> > +
> > +             #
> > +             # Actual testing
> > +             #
> > +
> > +             git switch --detach topic^0 &&
> > +
> > +             test_must_fail git -c merge.renameLimit=1 rebase upstream &&
> > +
> > +             git ls-files -u >actual &&
> > +             ! test_file_is_empty actual
>
> There is no 'test_file_is_empty' function, but because of the ! at the
> beginning of the line it didn't fail the test.

Oops, looks like I meant test_must_be_empty.

> The minimal fix would be to use 'test_file_not_empty' instead, but I
> wonder whether we should use 'test_line_count = 2' instead for a tad
> tighter check.

Makes sense; since this merged about half a year ago, I'll submit a
new patch to fix this.  Thanks for catching and pointing it out!



>
> > +     )
> > +'
> > +
> >  test_done
> >
> > base-commit: 1ffcbaa1a5f10c9f706314d77f88de20a4a498c2
> > --
> > gitgitgadget




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux