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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Nov 18, 2018, 14:18 UTC
Message-ID: <20181118141853.GR30222@szeder.dev>
In-Reply-To: <5c1aa82794da23077ef4ec4e836ee221c64162ef.1542496915.git.liu.denton@gmail.com>

On Sat, Nov 17, 2018 at 06:32:33PM -0500, Denton Liu wrote:

Show 22 quoted lines

> 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.

Show 5 quoted lines

> +		#
> +		# 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.

Show 25 quoted lines

> +'
> +
> +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.

Show 8 quoted lines

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