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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 08 Mar 2021 14:03:31 +0100

On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <[email-masked]>
>
> 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 <[email-masked]>
> ---
>  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.