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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 10, 2021, 13:26 UTC
Message-ID: <60c2130178831_b25b1208f5@natae.notmuch>
In-Reply-To: <b2436790-bbf0-2623-5e29-a6f6b4959974@gmail.com>

Phillip Wood wrote:

Show 6 quoted lines

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

That file is for diff3 conflict markers. The tests in this file are not.

> The commit message does not explain why a new file is better than
> adding this test to that file.

Because there's no file that is testing for this.

> There are already diff3 tests for checkout

This file is not doing diff3 tests.

As stated above, it's testing different *combinations* of merge.conflictstyle, diff3 is only *one* of the possibilities, another possibility is:

  git -c merge.conflictstyle=diff3 checkout -m --conflict=merge

That is *not* a diff3 test.

> > diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
> > new file mode 100755

Show 21 quoted lines

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

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

Cheers.

-- 
Felipe Contreras