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

[Felipe Contreras <felipe.contreras@gmail.com>]

Phillip Wood wrote:
> On 10/06/2021 14:26, Felipe Contreras wrote:
> > Phillip Wood wrote:
> >> 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.
> >>[...]
> >>> diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
> >>> new file mode 100755
> > 
> >>> +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.
> > 
> > But that would fail if there's a "command1 | command2".
> 
> I don't understand. What are you expecting content to contain?

Not a sequence of |.

> Why doesn't "\|+" fail in that case?

It would, perhaps "\|\|+" would be better, or maybe "\|{2,}".

> >> This is quite a weak
> >> test, something like "^|||||| " would be a stronger test for conflict
> >> markers
> > 
> > But that doesn't work in all the tests.
> 
> So test for what you actually expect, you don't need to have the same 
> check in all the tests if the expected output is different.

I don't need to, but it makes the tests simpler, and as you pointed out
in another comment: more tests are needed.

Perhaps once we know exactly what we want to test, and how to fix the
current issues it would make sense to revisit these.

-- 
Felipe Contreras