Re: [PATCH 07/11] t6428: new test for SKIP_WORKTREE handling and conflicts

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

 



On Mon, Mar 8, 2021 at 5:03 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>
>
> On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > If there is a conflict during a merge for a SKIP_WORKTREE entry, we
> > expect that file to be written to the working copy and have the
> > SKIP_WORKTREE bit cleared in the index.  If the user had manually
> > created a file in the working tree despite SKIP_WORKTREE being set, we
> > do not want to clobber their changes to that file, but want to move it
> > out of the way.  Add tests that check for these behaviors.
> >
> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> > ---
> >  t/t6428-merge-conflicts-sparse.sh | 158 ++++++++++++++++++++++++++++++
> >  1 file changed, 158 insertions(+)
> >  create mode 100755 t/t6428-merge-conflicts-sparse.sh
> >
> > diff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh
> > new file mode 100755
> > index 000000000000..1bb52ff6f38c
> > --- /dev/null
> > +++ b/t/t6428-merge-conflicts-sparse.sh
> > @@ -0,0 +1,158 @@
> > +#!/bin/sh
> > +
> > +test_description="merge cases"
> > +
> > +# The setup for all of them, pictorially, is:
> > +#
> > +#      A
> > +#      o
> > +#     / \
> > +#  O o   ?
> > +#     \ /
> > +#      o
> > +#      B
> > +#
> > +# To help make it easier to follow the flow of tests, they have been
> > +# divided into sections and each test will start with a quick explanation
> > +# of what commits O, A, and B contain.
> > +#
> > +# Notation:
> > +#    z/{b,c}   means  files z/b and z/c both exist
> > +#    x/d_1     means  file x/d exists with content d1.  (Purpose of the
> > +#                     underscore notation is to differentiate different
> > +#                     files that might be renamed into each other's paths.)
> > +
> > +. ./test-lib.sh
> > +. "$TEST_DIRECTORY"/lib-merge.sh
> > +
> > +
> > +# Testcase basic, conflicting changes in 'numerals'
> > +
> > +test_setup_numerals () {
> > +     test_create_repo numerals_$1 &&
> > +     (
> > +             cd numerals_$1 &&
> > +
> > +             >README &&
> > +             test_write_lines I II III >numerals &&
> > +             git add README numerals &&
> > +             test_tick &&
> > +             git commit -m "O" &&
>
> As an aside this could use the --printf option to test_commit I've got
> in next, but that's also a bit painful to use since you can't use
> test_write_lines.
>
> I've wanted to just support something like this for this use-case of
> using an existing file:
>
>     test_write_lines A B C D >lines &&
>     test_commit --add O lines &&
>
>
> > +
> > +             git branch O &&
> > +             git branch A &&
> > +             git branch B &&
> > +
> > +             git checkout A &&
> > +             test_write_lines I II III IIII >numerals &&
> > +             git add numerals &&
> > +             test_tick &&
> > +             git commit -m "A" &&
> > +
> > +             git checkout B &&
> > +             test_write_lines I II III IV >numerals &&
> > +             git add numerals &&
> > +             test_tick &&
> > +             git commit -m "B" &&
> > +
> > +             cat <<-EOF >expected-index &&
> > +             H README
> > +             M numerals
> > +             M numerals
> > +             M numerals
> > +             EOF
> > +
> > +             cat <<-EOF >expected-merge
> > +             I
> > +             II
> > +             III
> > +             <<<<<<< HEAD
> > +             IIII
> > +             =======
> > +             IV
> > +             >>>>>>> B^0
> > +             EOF
> > +
> > +     )
> > +}
> > +
> > +test_expect_merge_algorithm success failure 'conflicting entries written to worktree even if sparse' '
> > +     test_setup_numerals plain &&
>
> A small nit, but makes it easier to debug things: I think having what
> you have in "test_setup_numerals" above in a test_expect_success is a
> better pattern, then if it fails we can see where exactly.
>
> Then instead of calling "test_setup_numerals" here you'd do:
>
>     cp -R template plain &&
>
> To just copy over that existing setup template, or re-use it and have
> have the tests call a small helper to "test_when_finish" reset --hard
> back as appropriate.

I used to have tests follow that pattern, but Dscho objected
strenuously to it; see e.g.
https://lore.kernel.org/git/nycvar.QRO.7.76.6.1910141211130.46@xxxxxxxxxxxxxxxxx/

I went through and replaced most of the merge-recursive-related ones
to the current style to help him out.




[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