Re: [PATCH 1/7] test: add merge style config test

From: Phillip Wood <phillip.wood123@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>, git@vger.kernel.org
Cc: "David Aguilar" <davvid@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Sergey Organov" <sorganov@gmail.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Denton Liu" <liu.denton@gmail.com>
Subject: Re: [PATCH 1/7] test: add merge style config test
Date: Thu, 10 Jun 2021 10:18:22 +0100	[thread overview]
Message-ID: <b2436790-bbf0-2623-5e29-a6f6b4959974@gmail.com> (raw)
In-Reply-To: <20210609192842.696646-2-felipe.contreras@gmail.com>

On 09/06/2021 20:28, Felipe Contreras wrote:
> We want to test different combinations of merge.conflictstyle, and a new
> file is the best place to do that.

I'm not sure what this particular tests adds over the existing ones in 
t6427-diff3-conflict-markers.sh. The commit message does not explain why 
a new file is better than adding this test to that file. There are 
already diff3 tests for checkout so it ends up being confusing when a 
checkout test gets added to this file rather than with those tests later 
in the series because there is no longer a single location for diff3 
checkout tests.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>   t/t6440-config-conflict-markers.sh | 44 ++++++++++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>   create mode 100755 t/t6440-config-conflict-markers.sh
> 
> diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
> new file mode 100755
> index 0000000000..6952552c58
> --- /dev/null
> +++ b/t/t6440-config-conflict-markers.sh
> @@ -0,0 +1,44 @@
> +#!/bin/sh
> +
> +test_description='merge style conflict markers configurations'
> +
> +. ./test-lib.sh
> +
> +fill () {
> +	for i
> +	do
> +		echo "$i"
> +	done
> +}
> +
> +test_expect_success 'merge' '
> +	test_create_repo merge &&
> +	(
> +		cd merge &&
> +
> +		fill 1 2 3 >content &&
> +		git add content &&
> +		git commit -m base &&
> +
> +		git checkout -b r &&
> +		echo six >>content &&
> +		git commit -a -m right &&
> +
> +		git checkout master &&
> +		echo 7 >>content &&
> +		git commit -a -m left &&
> +
> +		test_must_fail git merge r &&
> +		! grep -E "\|+" content &&

! grep "|"  would be simpler and just as effective. This is quite a weak 
test, something like "^|||||| " would be a stronger test for conflict 
markers

Best Wishes

Phillip

> +		git reset --hard &&
> +		test_must_fail git -c merge.conflictstyle=diff3 merge r &&
> +		grep -E "\|+" content &&
> +
> +		git reset --hard &&
> +		test_must_fail git -c merge.conflictstyle=merge merge r &&
> +		! grep -E "\|+" content
> +	)
> +'
> +
> +test_done
> 