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

On 10/06/2021 17:47, Felipe Contreras wrote:

Phillip Wood wrote:

quoted

On 10/06/2021 14:26, Felipe Contreras wrote:

quoted

Phillip Wood wrote:

quoted

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

We want to test different combinations of merge.conflictstyle, and a new
file is the best place to do that.

[...]

quoted

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

quoted

quoted

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

quoted

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

quoted

quoted

quoted

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.