From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 11 Jun 2021 09:19:11 GMT
Subject: Re: [PATCH 1/7] test: add merge style config test
Message-ID: <0975b12c-82e1-344c-e09f-37f568c10514@gmail.com>
In-Reply-To: <60c2420a2dd73_b25b1208f1@natae.notmuch>

On 10/06/2021 17:47, Felipe Contreras wrote:
> 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,}".

The point of my original comment was that you do not need an ERE - 'grep 
"||"' matches the same set of lines as 'grep -E "\|\|+"'. As it is 
testing for conflict markers anchoring the pattern to the beginning of a 
line would probably be a good idea.

Best Wishes

Phillip

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