Re: [PATCH 04/10] Add new test to ensure git-merge handles pull.twohead and pull.octopus

[Junio C Hamano <gitster@pobox.com>]

Miklos Vajna <vmiklos@frugalware.org> writes:

> Test if the given strategies are used and test the case when multiple
> strategies are configured using a space separated list.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>  t/t7601-merge-pull-config.sh |   57 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 57 insertions(+), 0 deletions(-)
>  create mode 100755 t/t7601-merge-pull-config.sh
>
> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> new file mode 100755
> index 0000000..cc595ac
> --- /dev/null
> +++ b/t/t7601-merge-pull-config.sh
> @@ -0,0 +1,57 @@
> +#!/bin/sh
> +
> +test_description='git-merge
> +
> +Testing pull.* configuration parsing.'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	echo c0 >c0.c &&
> +	git add c0.c &&
> +	git commit -m c0 &&
> +	git tag c0 &&
> +	echo c1 >c1.c &&
> +	git add c1.c &&
> +	git commit -m c1 &&
> +	git tag c1 &&
> +	git reset --hard c0 &&
> +	echo c2 >c2.c &&
> +	git add c2.c &&
> +	git commit -m c2 &&
> +	git tag c2
> +	git reset --hard c0 &&
> +	echo c3 >c3.c &&
> +	git add c3.c &&
> +	git commit -m c3 &&
> +	git tag c3
> +'
> +
> +test_expect_success 'merge c1 with c2' '
> +	git reset --hard c1 &&

test that c0 and c1 do and c2 and c3 do not exist here, as it is cheap,
and otherwise you may end up chasing wild-goose when somebody breaks
git-reset.  No need to do so in later tests in this script, but it is a
cheap protection for yourself from others' mistakes ;-).

> +	git merge c2 &&
> +	test -e c1.c &&
> +	test -e c2.c
> +'

Nobody runs V7 that lacked "test -e" to run these test scripts, but you
expect them to be regular files at this point of the test, so the correct
way to spell these is with "test -f".

In general, you are better off training yourself to think if you can use
"test -f" before blindly using "test -e".

> +test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
> +	git reset --hard c1 &&
> +	git config pull.twohead ours &&
> +	git merge c2 &&
> +	test -e c1.c &&
> +	! test -e c2.c
> +'
> +
> +test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
> +	git reset --hard c1 &&
> +	git config pull.octopus "recursive" &&
> +	! git merge c2 c3

Is it because it should dump core, or is it because the command should
decline to work, gracefully failing with an error message and non-zero
exit status?  Use "test_must_fail" to check for the latter.

Don't you want to check how it fails and in what shape the command leaves
the work tree?  I am assuming that recursive sees more than one "remote"
head and declines to work without touching work tree nor the index, so if
that is what you expect, you should check for that.  Otherwise, a
regression that loses local changes will go unnoticed.

> +'
> +
> +test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octopus)' '
> +	git reset --hard c1 &&
> +	git config pull.octopus "recursive octopus" &&
> +	git merge c2 c3

Likewise, don't you want to check the result of the merge?  Not just
"merge exited with 0", but you would want to see that the HEAD has
advanced, it has the expected parents, there is no unexpected local
changes (because you did not have any when you started the merge), and it
has the expected tree contents.

> +'
> +
> +test_done
> -- 
> 1.5.6.rc0.dirty