Re: [PATCH v3 1/1] merge: add scissors line on merge conflict

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Sat, Nov 17, 2018 at 06:32:33PM -0500, Denton Liu wrote:
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 106148254d..0d3db34f08 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -247,6 +247,54 @@ test_expect_success 'merge --squash c3 with c7' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'merge c3 with c7 with commit.cleanup = scissors' '
> +	git config commit.cleanup scissors &&
> +	git reset --hard c3 &&
> +	test_must_fail git merge c7 &&
> +	cat result.9z >file &&
> +	git commit --no-edit -a &&
> +
> +	{
> +		cat <<-EOF
> +		Merge tag '"'"'c7'"'"'
> +
> +		# ------------------------ >8 ------------------------
> +		# Do not modify or remove the line above.
> +		# Everything below it will be ignored.

Note that these two lines of advice text are translated; see the
consequences below.

> +		#
> +		# Conflicts:
> +		#	file
> +		EOF
> +	} >expect &&

The {...} block is unnecessary, because there is only a single command
in there.

> +	git cat-file commit HEAD | sed -e '1,/^$/d' >actual &&

Please don't run git commands upstream of a pipe, because the pipe
hides their exit code.  Furthermore, put the sed script inside double
quotes, because the whole test is already in a single-quoted block.

I presume you wrote the test this way because you simply followed suit
of the previous test 'merge --squash c3 with c7', which did all the
same.  Bonus points for a preparatory patch that cleans up the
previous test ;)

> +	test_cmp expect actual

But most importantly, here 'test_cmp' compares translated advice text
as well, which fails in the GETTEXT_POISON build.  Use 'test_i18ncmp'
instead.

> +'
> +
> +test_expect_success 'merge c3 with c7 with --squash commit.cleanup = scissors' '
> +	git config commit.cleanup scissors &&
> +	git reset --hard c3 &&
> +	test_must_fail git merge --squash c7 &&
> +	cat result.9z >file &&
> +	git commit --no-edit -a &&
> +
> +	{
> +		cat <<-EOF
> +		Squashed commit of the following:
> +
> +		$(git show -s c7)
> +
> +		# ------------------------ >8 ------------------------
> +		# Do not modify or remove the line above.
> +		# Everything below it will be ignored.
> +		#
> +		# Conflicts:
> +		#	file
> +		EOF
> +	} >expect &&
> +	git cat-file commit HEAD | sed -e '1,/^$/d' >actual &&
> +	test_cmp expect actual

Likewise.

> +'
> +
>  test_debug 'git log --graph --decorate --oneline --all'
>  
>  test_expect_success 'merge c1 with c2 and c3' '
> -- 
> 2.19.1
>