Re: [PATCH 07/11] t6428: new test for SKIP_WORKTREE handling and conflicts
- From
Elijah Newren <newren@gmail.com>
- Date
- Mar 8, 2021, 20:52 UTC
- Message-ID
- <CABPp-BEZpq7yX0vOamXhDa3M6696YqrD_-UZE2GfFFJrORDdMQ@mail.gmail.com>
- In-Reply-To
- <874khlkdb0.fsf@evledraar.gmail.com>
On Mon, Mar 8, 2021 at 5:03 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 129 quoted lines
>
>
> On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > 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@gmail.com>
> > ---
> > 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@tvgsbejvaqbjf.bet/
I went through and replaced most of the merge-recursive-related ones to the current style to help him out.