All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
       [not found] <CABPp-BEcTKaPPUOVqTRUAW+LBBySCK0dgx1J66hYB30yMasK_Q@mail.gmail.com/>
@ 2018-05-26  1:09 ` Elijah Newren
  2018-05-26 23:58   ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Elijah Newren @ 2018-05-26  1:09 UTC (permalink / raw)
  To: gitster; +Cc: git, szeder.dev, Elijah Newren

On Thu, May 24, 2018 at 6:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>>> -            test 2 = $(git ls-files -s | wc -l) &&
>>> -            test 2 = $(git ls-files -u | wc -l) &&
>>> -            test 2 = $(git ls-files -o | wc -l) &&
>>
>> Here 'git ls-files -o' should have listed two untracked files ...
>>
>>> +            git ls-files -s >out &&
>>> +            test_line_count = 2 out &&
>>> +            git ls-files -u >out &&
>>> +            test_line_count = 2 out &&
>>> +            git ls-files -o >out &&
>>> +            test_line_count = 3 out &&
>>
>> ... but now you expect it to list three.  I was about to point out the
>> typo, but then noticed that you expect it to list one more untracked
>> file than before in all subsequent tests...  now that can't be just a
>> typo, can it?
>>
>> Please mention in the commit message that when using an intermediate
>> file to store the output, 'git ls-files -o' will list that file, too,
>> that's why the number of expected untracked files had to be adjusted;
>> so future readers won't have to figure this out themselves.
>
> I'd expect that a reader of the commit who cares enough to bother to
> wonder by looking at the patch and seeing that 2 became 3 would know
> why already.  And a reader of the resulting file would not know that
> the 3 used to be 2, and won't be helped by "we used to count to 2,
> now we have 'out' also counted" that much, especially in the commit
> log message.  What would help the latter would be to name which
> three paths we expect to see in the comment (or test against the
> exact list of paths, instead of using test_line_count).
>
>> An alternative to consider would be to add a .gitignore file in the
>> initial commit to ignore 'out', then the number of untracked files
>> don't have to be adjusted.
>
> I think that is a preferred solution that we've used in ls-files and
> status tests successfully.

...except that if we add a .gitignore to each initial commit (we use
test_create_repo for nearly every test to keep them separable meaning
we'd have to do this many times), then four lines above we have to
adjust the number of expected tracked files.  And, for it to work,
we'd have to add an --exclude-standard flag to ls-files -o.

I can make that change if you both want it, but it seems like it's
actually making it harder to follow the changes rather than easier.

-- >8 --
Subject: [PATCH] fixup! t6036, t6042: use test_line_count instead of wc -l

---

and here's what it looks like which you can apply as a fixup.  I think it makes things worse, though.

 t/t6036-recursive-corner-cases.sh    | 81 +++++++++++++-----------
 t/t6042-merge-rename-corner-cases.sh | 93 +++++++++++++++-------------
 2 files changed, 95 insertions(+), 79 deletions(-)

diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 3e659cff28..06944a8c0a 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -25,6 +25,7 @@ test_expect_success 'setup basic criss-cross + rename with no modifications' '
 	(
 		cd basic-rename &&
 
+		echo out >.gitignore &&
 		ten="0 1 2 3 4 5 6 7 8 9" &&
 		for i in $ten
 		do
@@ -34,7 +35,7 @@ test_expect_success 'setup basic criss-cross + rename with no modifications' '
 		do
 			echo line $i in another sample file
 		done >two &&
-		git add one two &&
+		git add .gitignore one two &&
 		test_tick && git commit -m initial &&
 
 		git branch L1 &&
@@ -66,11 +67,11 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
 		test_must_fail git merge -s recursive R2^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 3 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 2 out &&
 
 		test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
 		test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
@@ -97,6 +98,7 @@ test_expect_success 'setup criss-cross + rename merges with basic modification'
 	(
 		cd rename-modify &&
 
+		echo out >.gitignore &&
 		ten="0 1 2 3 4 5 6 7 8 9" &&
 		for i in $ten
 		do
@@ -106,7 +108,7 @@ test_expect_success 'setup criss-cross + rename merges with basic modification'
 		do
 			echo line $i in another sample file
 		done >two &&
-		git add one two &&
+		git add .gitignore one two &&
 		test_tick && git commit -m initial &&
 
 		git branch L1 &&
@@ -139,11 +141,11 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
 		test_must_fail git merge -s recursive R2^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 3 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 2 out &&
 
 		test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
 		test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
@@ -178,8 +180,9 @@ test_expect_success 'setup differently handled merges of rename/add conflict' '
 	(
 		cd rename-add &&
 
+		echo out >.gitignore &&
 		printf "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n" >a &&
-		git add a &&
+		git add .gitignore a &&
 		test_tick && git commit -m A &&
 
 		git branch B &&
@@ -218,11 +221,11 @@ test_expect_success 'git detects differently handled merges conflict' '
 		test_must_fail git merge -s recursive E^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 3 out &&
+		test_line_count = 4 out &&
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse :2:new_a) = $(git rev-parse D:new_a) &&
 		test $(git rev-parse :3:new_a) = $(git rev-parse E:new_a) &&
@@ -376,8 +379,9 @@ test_expect_success 'setup differently handled merges of directory/file conflict
 	(
 		cd directory-file &&
 
+		echo out >.gitignore &&
 		>ignore-me &&
-		git add ignore-me &&
+		git add .gitignore ignore-me &&
 		test_tick &&
 		git commit -m A &&
 		git tag A &&
@@ -437,11 +441,11 @@ test_expect_success 'merge of D & E1 fails but has appropriate contents' '
 		test_must_fail git merge -s recursive E1^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 1 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
 		test $(git rev-parse :2:a) = $(git rev-parse B:a)
@@ -457,11 +461,11 @@ test_expect_success 'merge of E1 & D fails but has appropriate contents' '
 		test_must_fail git merge -s recursive D^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 1 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
 		test $(git rev-parse :3:a) = $(git rev-parse B:a)
@@ -477,11 +481,11 @@ test_expect_success 'merge of D & E2 fails but has appropriate contents' '
 		test_must_fail git merge -s recursive E2^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 4 out &&
+		test_line_count = 5 out &&
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
-		git ls-files -o >out &&
-		test_line_count = 2 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse :2:a) = $(git rev-parse B:a) &&
 		test $(git rev-parse :3:a/file) = $(git rev-parse E2:a/file) &&
@@ -501,11 +505,11 @@ test_expect_success 'merge of E2 & D fails but has appropriate contents' '
 		test_must_fail git merge -s recursive D^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 4 out &&
+		test_line_count = 5 out &&
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
-		git ls-files -o >out &&
-		test_line_count = 2 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse :3:a) = $(git rev-parse B:a) &&
 		test $(git rev-parse :2:a/file) = $(git rev-parse E2:a/file) &&
@@ -560,8 +564,9 @@ test_expect_success 'setup rename/rename(1to2)/modify followed by what looks lik
 	(
 		cd rename-squared-squared &&
 
+		echo out >.gitignore &&
 		printf "1\n2\n3\n4\n5\n6\n" >a &&
-		git add a &&
+		git add .gitignore a &&
 		git commit -m A &&
 		git tag A &&
 
@@ -600,11 +605,11 @@ test_expect_success 'handle rename/rename(1to2)/modify followed by what looks li
 		git merge -s recursive E^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 1 out &&
+		test_line_count = 2 out &&
 		git ls-files -u >out &&
 		test_line_count = 0 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse HEAD:newname) = $(git rev-parse E:newname)
 	)
@@ -637,8 +642,9 @@ test_expect_success 'setup criss-cross + rename/rename/add-source + modify/modif
 	(
 		cd rename-rename-add-source &&
 
+		echo out >.gitignore &&
 		printf "lots\nof\nwords\nand\ncontent\n" >a &&
-		git add a &&
+		git add .gitignore a &&
 		git commit -m A &&
 		git tag A &&
 
@@ -682,11 +688,11 @@ test_expect_failure 'detect rename/rename/add-source for virtual merge-base' '
 		git merge -s recursive E^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 3 out &&
+		test_line_count = 4 out &&
 		git ls-files -u >out &&
 		test_line_count = 0 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse HEAD:b) = $(git rev-parse A:a) &&
 		test $(git rev-parse HEAD:c) = $(git rev-parse A:a) &&
@@ -719,8 +725,9 @@ test_expect_success 'setup criss-cross+rename/rename/add-dest + simple modify' '
 	(
 		cd rename-rename-add-dest &&
 
+		echo out >.gitignore &&
 		>a &&
-		git add a &&
+		git add .gitignore a &&
 		git commit -m A &&
 		git tag A &&
 
@@ -759,11 +766,11 @@ test_expect_success 'virtual merge base handles rename/rename(1to2)/add-dest' '
 		git merge -s recursive E^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 0 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse HEAD:a) = $(git rev-parse A:a) &&
 		test $(git rev-parse HEAD:c) = $(git rev-parse E:c)
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index b76da8fcdf..a330ced7da 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -90,9 +90,10 @@ test_expect_success 'setup resolvable conflict missed if rename missed' '
 	(
 		cd break-detection-1 &&
 
+		echo out >.gitignore &&
 		printf "1\n2\n3\n4\n5\n" >a &&
 		echo foo >b &&
-		git add a b &&
+		git add .gitignore a b &&
 		git commit -m A &&
 		git tag A &&
 
@@ -117,11 +118,11 @@ test_expect_failure 'conflict caused if rename not detected' '
 		git merge -s recursive B^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 3 out &&
+		test_line_count = 4 out &&
 		git ls-files -u >out &&
 		test_line_count = 0 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test_line_count = 6 c &&
 		test $(git rev-parse HEAD:a) = $(git rev-parse B:a) &&
@@ -181,8 +182,9 @@ test_expect_success 'setup undetected rename/add-source causes data loss' '
 	(
 		cd break-detection-3 &&
 
+		echo out >.gitignore &&
 		printf "1\n2\n3\n4\n5\n" >a &&
-		git add a &&
+		git add .gitignore a &&
 		git commit -m A &&
 		git tag A &&
 
@@ -207,11 +209,11 @@ test_expect_failure 'detect rename/add-source and preserve all data' '
 		git merge -s recursive C^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test -f a &&
 		test -f b &&
@@ -230,11 +232,11 @@ test_expect_failure 'detect rename/add-source and preserve all data, merge other
 		git merge -s recursive B^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test -f a &&
 		test -f b &&
@@ -249,8 +251,9 @@ test_expect_success 'setup content merge + rename/directory conflict' '
 	(
 		cd rename-directory-1 &&
 
+		echo out >.gitignore &&
 		printf "1\n2\n3\n4\n5\n6\n" >file &&
-		git add file &&
+		git add .gitignore file &&
 		test_tick &&
 		git commit -m base &&
 		git tag base &&
@@ -289,11 +292,11 @@ test_expect_success 'rename/directory conflict + clean content merge' '
 		test_must_fail git merge -s recursive right^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 1 out &&
-		git ls-files -o >out &&
-		test_line_count = 2 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 1 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:file >>expect &&
@@ -320,11 +323,11 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 		test_must_fail git merge -s recursive right^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 4 out &&
+		test_line_count = 5 out &&
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
-		git ls-files -o >out &&
-		test_line_count = 2 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 1 out &&
 
 		git cat-file -p left-conflict:newfile >left &&
 		git cat-file -p base:file    >base &&
@@ -350,9 +353,10 @@ test_expect_success 'setup content merge + rename/directory conflict w/ disappea
 	(
 		cd rename-directory-2 &&
 
+		echo out >.gitignore &&
 		mkdir sub &&
 		printf "1\n2\n3\n4\n5\n6\n" >sub/file &&
-		git add sub/file &&
+		git add .gitignore sub/file &&
 		test_tick &&
 		git commit -m base &&
 		git tag base &&
@@ -383,11 +387,11 @@ test_expect_success 'disappearing dir in rename/directory conflict handled' '
 		git merge -s recursive right^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 1 out &&
+		test_line_count = 2 out &&
 		git ls-files -u >out &&
 		test_line_count = 0 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:sub/file >>expect &&
@@ -415,9 +419,10 @@ test_expect_success 'setup rename/rename (2to1) + modify/modify' '
 	(
 		cd rename-rename-2to1 &&
 
+		echo out >.gitignore &&
 		printf "1\n2\n3\n4\n5\n" >a &&
 		printf "5\n4\n3\n2\n1\n" >b &&
-		git add a b &&
+		git add .gitignore a b &&
 		git commit -m A &&
 		git tag A &&
 
@@ -445,13 +450,13 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '
 		test_i18ngrep "CONFLICT (rename/rename)" out &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
+		test_line_count = 3 out &&
 		git ls-files -u >out &&
 		test_line_count = 2 out &&
 		git ls-files -u c >out &&
 		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 3 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 2 out &&
 
 		test ! -f a &&
 		test ! -f b &&
@@ -472,8 +477,9 @@ test_expect_success 'setup simple rename/rename (1to2) conflict' '
 	(
 		cd rename-rename-1to2 &&
 
+		echo out >.gitignore &&
 		echo stuff >a &&
-		git add a &&
+		git add .gitignore a &&
 		test_tick &&
 		git commit -m A &&
 		git tag A &&
@@ -499,11 +505,11 @@ test_expect_success 'merge has correct working tree contents' '
 		test_must_fail git merge -s recursive B^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 3 out &&
+		test_line_count = 4 out &&
 		git ls-files -u >out &&
 		test_line_count = 3 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
 		test $(git rev-parse :3:b) = $(git rev-parse A:a) &&
@@ -527,8 +533,9 @@ test_expect_success 'setup rename/rename(1to2)/add-source conflict' '
 	(
 		cd rename-rename-1to2-add-source-1 &&
 
+		echo out >.gitignore &&
 		printf "1\n2\n3\n4\n5\n6\n7\n" >a &&
-		git add a &&
+		git add .gitignore a &&
 		git commit -m A &&
 		git tag A &&
 
@@ -553,9 +560,9 @@ test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge'
 		test_must_fail git merge -s recursive C^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 4 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		test_line_count = 5 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse 3:a) = $(git rev-parse C:a) &&
 		test $(git rev-parse 1:a) = $(git rev-parse A:a) &&
@@ -573,8 +580,9 @@ test_expect_success 'setup rename/rename(1to2)/add-source resolvable conflict' '
 	(
 		cd rename-rename-1to2-add-source-2 &&
 
+		echo out >.gitignore &&
 		>a &&
-		git add a &&
+		git add .gitignore a &&
 		test_tick &&
 		git commit -m base &&
 		git tag A &&
@@ -601,9 +609,9 @@ test_expect_failure 'rename/rename/add-source still tracks new a file' '
 		git merge -s recursive B^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 1 out &&
+		test_line_count = 3 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 0 out &&
 
 		test $(git rev-parse HEAD:a) = $(git rev-parse C:a) &&
 		test $(git rev-parse HEAD:b) = $(git rev-parse A:a)
@@ -615,8 +623,9 @@ test_expect_success 'setup rename/rename(1to2)/add-dest conflict' '
 	(
 		cd rename-rename-1to2-add-dest &&
 
+		echo out >.gitignore &&
 		echo stuff >a &&
-		git add a &&
+		git add .gitignore a &&
 		test_tick &&
 		git commit -m base &&
 		git tag A &&
@@ -645,13 +654,13 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
 		test_must_fail git merge -s recursive B^0 &&
 
 		git ls-files -s >out &&
-		test_line_count = 5 out &&
+		test_line_count = 6 out &&
 		git ls-files -u b >out &&
 		test_line_count = 2 out &&
 		git ls-files -u c >out &&
 		test_line_count = 2 out &&
-		git ls-files -o >out &&
-		test_line_count = 5 out &&
+		git ls-files -o --exclude-standard >out &&
+		test_line_count = 4 out &&
 
 		test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
 		test $(git rev-parse :2:b) = $(git rev-parse C:b) &&
-- 
2.17.0.844.g1ca50c8745.dirty


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-26  1:09 ` [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l Elijah Newren
@ 2018-05-26 23:58   ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2018-05-26 23:58 UTC (permalink / raw)
  To: Elijah Newren; +Cc: git, szeder.dev

Elijah Newren <newren@gmail.com> writes:

>> I'd expect that a reader of the commit who cares enough to bother to
>> wonder by looking at the patch and seeing that 2 became 3 would know
>> why already.  And a reader of the resulting file would not know that
>> the 3 used to be 2, and won't be helped by "we used to count to 2,
>> now we have 'out' also counted" that much, especially in the commit
>> log message.  What would help the latter would be to name which
>> three paths we expect to see in the comment (or test against the
>> exact list of paths, instead of using test_line_count).
>>
>>> An alternative to consider would be to add a .gitignore file in the
>>> initial commit to ignore 'out', then the number of untracked files
>>> don't have to be adjusted.
>>
>> I think that is a preferred solution that we've used in ls-files and
>> status tests successfully.
>
> ...except that if we add a .gitignore to each initial commit (we use
> test_create_repo for nearly every test to keep them separable meaning
> we'd have to do this many times), then four lines above we have to
> adjust the number of expected tracked files.  And, for it to work,
> we'd have to add an --exclude-standard flag to ls-files -o.

Yeah, unless the original planned to use the .gitignore mechanism,
converting it to use it now will become noisy.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-25  1:17     ` Junio C Hamano
@ 2018-05-26  0:44       ` Elijah Newren
  0 siblings, 0 replies; 8+ messages in thread
From: Elijah Newren @ 2018-05-26  0:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, Git Mailing List

On Thu, May 24, 2018 at 6:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>>> -            test 2 = $(git ls-files -s | wc -l) &&
>>> -            test 2 = $(git ls-files -u | wc -l) &&
>>> -            test 2 = $(git ls-files -o | wc -l) &&
>>
>> Here 'git ls-files -o' should have listed two untracked files ...
>>
>>> +            git ls-files -s >out &&
>>> +            test_line_count = 2 out &&
>>> +            git ls-files -u >out &&
>>> +            test_line_count = 2 out &&
>>> +            git ls-files -o >out &&
>>> +            test_line_count = 3 out &&
>>
>> ... but now you expect it to list three.  I was about to point out the
>> typo, but then noticed that you expect it to list one more untracked
>> file than before in all subsequent tests...  now that can't be just a
>> typo, can it?
>>
>> Please mention in the commit message that when using an intermediate
>> file to store the output, 'git ls-files -o' will list that file, too,
>> that's why the number of expected untracked files had to be adjusted;
>> so future readers won't have to figure this out themselves.
>
> I'd expect that a reader of the commit who cares enough to bother to
> wonder by looking at the patch and seeing that 2 became 3 would know
> why already.  And a reader of the resulting file would not know that
> the 3 used to be 2, and won't be helped by "we used to count to 2,
> now we have 'out' also counted" that much, especially in the commit
> log message.  What would help the latter would be to name which
> three paths we expect to see in the comment (or test against the
> exact list of paths, instead of using test_line_count).
>
>> An alternative to consider would be to add a .gitignore file in the
>> initial commit to ignore 'out', then the number of untracked files
>> don't have to be adjusted.
>
> I think that is a preferred solution that we've used in ls-files and
> status tests successfully.

...except that if we add a .gitignore to each initial commit (we use
test_create_repo for nearly every test to keep them separable meaning
we'd have to do this many times), then four lines above we have to
adjust the number of expected tracked files.  And, for it to work,
we'd have to add an --exclude-standard flag to ls-files -o.

I can make that change if you both want it, but it seems like it's
actually making it harder to follow the changes rather than easier.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-24 10:05   ` SZEDER Gábor
  2018-05-24 10:36     ` SZEDER Gábor
  2018-05-24 17:03     ` Elijah Newren
@ 2018-05-25  1:17     ` Junio C Hamano
  2018-05-26  0:44       ` Elijah Newren
  2 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2018-05-25  1:17 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Elijah Newren, git

SZEDER Gábor <szeder.dev@gmail.com> writes:

>> -		test 2 = $(git ls-files -s | wc -l) &&
>> -		test 2 = $(git ls-files -u | wc -l) &&
>> -		test 2 = $(git ls-files -o | wc -l) &&
>
> Here 'git ls-files -o' should have listed two untracked files ...
>
>> +		git ls-files -s >out &&
>> +		test_line_count = 2 out &&
>> +		git ls-files -u >out &&
>> +		test_line_count = 2 out &&
>> +		git ls-files -o >out &&
>> +		test_line_count = 3 out &&
>
> ... but now you expect it to list three.  I was about to point out the
> typo, but then noticed that you expect it to list one more untracked
> file than before in all subsequent tests...  now that can't be just a
> typo, can it?
>
> Please mention in the commit message that when using an intermediate
> file to store the output, 'git ls-files -o' will list that file, too,
> that's why the number of expected untracked files had to be adjusted;
> so future readers won't have to figure this out themselves.

I'd expect that a reader of the commit who cares enough to bother to
wonder by looking at the patch and seeing that 2 became 3 would know
why already.  And a reader of the resulting file would not know that
the 3 used to be 2, and won't be helped by "we used to count to 2,
now we have 'out' also counted" that much, especially in the commit
log message.  What would help the latter would be to name which
three paths we expect to see in the comment (or test against the
exact list of paths, instead of using test_line_count).

> An alternative to consider would be to add a .gitignore file in the
> initial commit to ignore 'out', then the number of untracked files
> don't have to be adjusted.

I think that is a preferred solution that we've used in ls-files and
status tests successfully.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-24 10:05   ` SZEDER Gábor
  2018-05-24 10:36     ` SZEDER Gábor
@ 2018-05-24 17:03     ` Elijah Newren
  2018-05-25  1:17     ` Junio C Hamano
  2 siblings, 0 replies; 8+ messages in thread
From: Elijah Newren @ 2018-05-24 17:03 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Git Mailing List, Junio C Hamano

On Thu, May 24, 2018 at 3:05 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>> --- a/t/t6036-recursive-corner-cases.sh
>> +++ b/t/t6036-recursive-corner-cases.sh
>> @@ -65,9 +65,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
>>
>>               test_must_fail git merge -s recursive R2^0 &&
>>
>> -             test 2 = $(git ls-files -s | wc -l) &&
>> -             test 2 = $(git ls-files -u | wc -l) &&
>> -             test 2 = $(git ls-files -o | wc -l) &&
>
> Here 'git ls-files -o' should have listed two untracked files ...
>
>> +             git ls-files -s >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -u >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -o >out &&
>> +             test_line_count = 3 out &&
>
> ... but now you expect it to list three.  I was about to point out the
> typo, but then noticed that you expect it to list one more untracked
> file than before in all subsequent tests...  now that can't be just a
> typo, can it?
>
> Please mention in the commit message that when using an intermediate
> file to store the output, 'git ls-files -o' will list that file, too,
> that's why the number of expected untracked files had to be adjusted;
> so future readers won't have to figure this out themselves.

How does adding the following to the commit message sound?

    Changing to use test_line_count means using an intermediate file to store
    the output, which means that 'git ls-files -o' will have an additional
    file to list, which means that the number of lines expected in some tests
    will therefore change as well (unless the same intermediate file was used
    to capture the output of a previous command).

I'll include that in my next roll of the series after waiting for any
other fixups folks point out.  Sorry for the trouble.

As always, thanks for taking a look!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-24 10:05   ` SZEDER Gábor
@ 2018-05-24 10:36     ` SZEDER Gábor
  2018-05-24 17:03     ` Elijah Newren
  2018-05-25  1:17     ` Junio C Hamano
  2 siblings, 0 replies; 8+ messages in thread
From: SZEDER Gábor @ 2018-05-24 10:36 UTC (permalink / raw)
  To: Elijah Newren; +Cc: SZEDER Gábor, Git mailing list, Junio C Hamano

On Thu, May 24, 2018 at 12:05 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>> ---
>>  t/t6036-recursive-corner-cases.sh    | 102 ++++++++++++++++++---------
>>  t/t6042-merge-rename-corner-cases.sh |  99 +++++++++++++++++---------
>>  2 files changed, 134 insertions(+), 67 deletions(-)
>>
>> diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
>> index cfe6a99771..3e659cff28 100755
>> --- a/t/t6036-recursive-corner-cases.sh
>> +++ b/t/t6036-recursive-corner-cases.sh
>> @@ -65,9 +65,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
>>
>>               test_must_fail git merge -s recursive R2^0 &&
>>
>> -             test 2 = $(git ls-files -s | wc -l) &&
>> -             test 2 = $(git ls-files -u | wc -l) &&
>> -             test 2 = $(git ls-files -o | wc -l) &&
>
> Here 'git ls-files -o' should have listed two untracked files ...
>
>> +             git ls-files -s >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -u >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -o >out &&
>> +             test_line_count = 3 out &&
>
> ... but now you expect it to list three.  I was about to point out the
> typo, but then noticed that you expect it to list one more untracked
> file than before in all subsequent tests...

Hrm, not all, actually, because there is this one exception:

>> @@ -426,10 +444,14 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '
>>               test_must_fail git merge -s recursive C^0 >out &&

Note that the file 'out' is created here ...

>>               test_i18ngrep "CONFLICT (rename/rename)" out &&
>>
>> -             test 2 -eq $(git ls-files -s | wc -l) &&
>> -             test 2 -eq $(git ls-files -u | wc -l) &&
>> -             test 2 -eq $(git ls-files -u c | wc -l) &&
>> -             test 3 -eq $(git ls-files -o | wc -l) &&

... so this  'git ls-files -o' already lists it as well, ...

>> +             git ls-files -s >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -u >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -u c >out &&
>> +             test_line_count = 2 out &&
>> +             git ls-files -o >out &&
>> +             test_line_count = 3 out &&

... therefore the number of untracked files here remains unchanged.

>>
>>               test ! -f a &&
>>               test ! -f b &&

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-24  7:04 ` [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l Elijah Newren
@ 2018-05-24 10:05   ` SZEDER Gábor
  2018-05-24 10:36     ` SZEDER Gábor
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: SZEDER Gábor @ 2018-05-24 10:05 UTC (permalink / raw)
  To: Elijah Newren; +Cc: SZEDER Gábor, git, gitster


> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  t/t6036-recursive-corner-cases.sh    | 102 ++++++++++++++++++---------
>  t/t6042-merge-rename-corner-cases.sh |  99 +++++++++++++++++---------
>  2 files changed, 134 insertions(+), 67 deletions(-)
> 
> diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
> index cfe6a99771..3e659cff28 100755
> --- a/t/t6036-recursive-corner-cases.sh
> +++ b/t/t6036-recursive-corner-cases.sh
> @@ -65,9 +65,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
>  
>  		test_must_fail git merge -s recursive R2^0 &&
>  
> -		test 2 = $(git ls-files -s | wc -l) &&
> -		test 2 = $(git ls-files -u | wc -l) &&
> -		test 2 = $(git ls-files -o | wc -l) &&

Here 'git ls-files -o' should have listed two untracked files ...

> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 3 out &&

... but now you expect it to list three.  I was about to point out the
typo, but then noticed that you expect it to list one more untracked
file than before in all subsequent tests...  now that can't be just a
typo, can it?

Please mention in the commit message that when using an intermediate
file to store the output, 'git ls-files -o' will list that file, too,
that's why the number of expected untracked files had to be adjusted;
so future readers won't have to figure this out themselves.

An alternative to consider would be to add a .gitignore file in the
initial commit to ignore 'out', then the number of untracked files
don't have to be adjusted.


[I have no further comments, but leave the rest of the patch below, so
anyone can readily check the increased number of untracked files in
subsequent tests.]


>  		test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
>  		test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
> @@ -135,9 +138,12 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
>  
>  		test_must_fail git merge -s recursive R2^0 &&
>  
> -		test 2 = $(git ls-files -s | wc -l) &&
> -		test 2 = $(git ls-files -u | wc -l) &&
> -		test 2 = $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 3 out &&
>  
>  		test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
>  		test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
> @@ -211,9 +217,12 @@ test_expect_success 'git detects differently handled merges conflict' '
>  
>  		test_must_fail git merge -s recursive E^0 &&
>  
> -		test 3 = $(git ls-files -s | wc -l) &&
> -		test 3 = $(git ls-files -u | wc -l) &&
> -		test 0 = $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse :2:new_a) = $(git rev-parse D:new_a) &&
>  		test $(git rev-parse :3:new_a) = $(git rev-parse E:new_a) &&
> @@ -297,8 +306,10 @@ test_expect_success 'git detects conflict merging criss-cross+modify/delete' '
>  
>  		test_must_fail git merge -s recursive E^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 2 -eq $(git ls-files -u | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
>  
>  		test $(git rev-parse :1:file) = $(git rev-parse master:file) &&
>  		test $(git rev-parse :2:file) = $(git rev-parse B:file)
> @@ -314,8 +325,10 @@ test_expect_success 'git detects conflict merging criss-cross+modify/delete, rev
>  
>  		test_must_fail git merge -s recursive D^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 2 -eq $(git ls-files -u | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
>  
>  		test $(git rev-parse :1:file) = $(git rev-parse master:file) &&
>  		test $(git rev-parse :3:file) = $(git rev-parse B:file)
> @@ -423,9 +436,12 @@ test_expect_success 'merge of D & E1 fails but has appropriate contents' '
>  
>  		test_must_fail git merge -s recursive E1^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 1 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 1 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
>  		test $(git rev-parse :2:a) = $(git rev-parse B:a)
> @@ -440,9 +456,12 @@ test_expect_success 'merge of E1 & D fails but has appropriate contents' '
>  
>  		test_must_fail git merge -s recursive D^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 1 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 1 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
>  		test $(git rev-parse :3:a) = $(git rev-parse B:a)
> @@ -457,9 +476,12 @@ test_expect_success 'merge of D & E2 fails but has appropriate contents' '
>  
>  		test_must_fail git merge -s recursive E2^0 &&
>  
> -		test 4 -eq $(git ls-files -s | wc -l) &&
> -		test 3 -eq $(git ls-files -u | wc -l) &&
> -		test 1 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 4 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 2 out &&
>  
>  		test $(git rev-parse :2:a) = $(git rev-parse B:a) &&
>  		test $(git rev-parse :3:a/file) = $(git rev-parse E2:a/file) &&
> @@ -478,9 +500,12 @@ test_expect_success 'merge of E2 & D fails but has appropriate contents' '
>  
>  		test_must_fail git merge -s recursive D^0 &&
>  
> -		test 4 -eq $(git ls-files -s | wc -l) &&
> -		test 3 -eq $(git ls-files -u | wc -l) &&
> -		test 1 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 4 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 2 out &&
>  
>  		test $(git rev-parse :3:a) = $(git rev-parse B:a) &&
>  		test $(git rev-parse :2:a/file) = $(git rev-parse E2:a/file) &&
> @@ -574,9 +599,12 @@ test_expect_success 'handle rename/rename(1to2)/modify followed by what looks li
>  
>  		git merge -s recursive E^0 &&
>  
> -		test 1 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 1 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 0 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse HEAD:newname) = $(git rev-parse E:newname)
>  	)
> @@ -653,9 +681,12 @@ test_expect_failure 'detect rename/rename/add-source for virtual merge-base' '
>  
>  		git merge -s recursive E^0 &&
>  
> -		test 3 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 0 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse HEAD:b) = $(git rev-parse A:a) &&
>  		test $(git rev-parse HEAD:c) = $(git rev-parse A:a) &&
> @@ -727,9 +758,12 @@ test_expect_success 'virtual merge base handles rename/rename(1to2)/add-dest' '
>  
>  		git merge -s recursive E^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 0 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse HEAD:a) = $(git rev-parse A:a) &&
>  		test $(git rev-parse HEAD:c) = $(git rev-parse E:c)
> diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
> index bec0192c3b..b76da8fcdf 100755
> --- a/t/t6042-merge-rename-corner-cases.sh
> +++ b/t/t6042-merge-rename-corner-cases.sh
> @@ -116,9 +116,12 @@ test_expect_failure 'conflict caused if rename not detected' '
>  		git checkout -q C^0 &&
>  		git merge -s recursive B^0 &&
>  
> -		test 3 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 0 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test_line_count = 6 c &&
>  		test $(git rev-parse HEAD:a) = $(git rev-parse B:a) &&
> @@ -203,9 +206,12 @@ test_expect_failure 'detect rename/add-source and preserve all data' '
>  
>  		git merge -s recursive C^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 2 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test -f a &&
>  		test -f b &&
> @@ -223,9 +229,12 @@ test_expect_failure 'detect rename/add-source and preserve all data, merge other
>  
>  		git merge -s recursive B^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 2 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test -f a &&
>  		test -f b &&
> @@ -279,9 +288,12 @@ test_expect_success 'rename/directory conflict + clean content merge' '
>  
>  		test_must_fail git merge -s recursive right^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 1 -eq $(git ls-files -u | wc -l) &&
> -		test 1 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 1 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 2 out &&
>  
>  		echo 0 >expect &&
>  		git cat-file -p base:file >>expect &&
> @@ -307,9 +319,12 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
>  
>  		test_must_fail git merge -s recursive right^0 &&
>  
> -		test 4 -eq $(git ls-files -s | wc -l) &&
> -		test 3 -eq $(git ls-files -u | wc -l) &&
> -		test 1 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 4 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 2 out &&
>  
>  		git cat-file -p left-conflict:newfile >left &&
>  		git cat-file -p base:file    >base &&
> @@ -367,9 +382,12 @@ test_expect_success 'disappearing dir in rename/directory conflict handled' '
>  
>  		git merge -s recursive right^0 &&
>  
> -		test 1 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 1 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 0 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		echo 0 >expect &&
>  		git cat-file -p base:sub/file >>expect &&
> @@ -426,10 +444,14 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '
>  		test_must_fail git merge -s recursive C^0 >out &&
>  		test_i18ngrep "CONFLICT (rename/rename)" out &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 2 -eq $(git ls-files -u | wc -l) &&
> -		test 2 -eq $(git ls-files -u c | wc -l) &&
> -		test 3 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u c >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 3 out &&
>  
>  		test ! -f a &&
>  		test ! -f b &&
> @@ -476,9 +498,12 @@ test_expect_success 'merge has correct working tree contents' '
>  
>  		test_must_fail git merge -s recursive B^0 &&
>  
> -		test 3 -eq $(git ls-files -s | wc -l) &&
> -		test 3 -eq $(git ls-files -u | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -u >out &&
> +		test_line_count = 3 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
>  		test $(git rev-parse :3:b) = $(git rev-parse A:a) &&
> @@ -527,8 +552,10 @@ test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge'
>  
>  		test_must_fail git merge -s recursive C^0 &&
>  
> -		test 4 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 4 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse 3:a) = $(git rev-parse C:a) &&
>  		test $(git rev-parse 1:a) = $(git rev-parse A:a) &&
> @@ -573,8 +600,10 @@ test_expect_failure 'rename/rename/add-source still tracks new a file' '
>  		git checkout C^0 &&
>  		git merge -s recursive B^0 &&
>  
> -		test 2 -eq $(git ls-files -s | wc -l) &&
> -		test 0 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 1 out &&
>  
>  		test $(git rev-parse HEAD:a) = $(git rev-parse C:a) &&
>  		test $(git rev-parse HEAD:b) = $(git rev-parse A:a)
> @@ -615,10 +644,14 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
>  		git checkout C^0 &&
>  		test_must_fail git merge -s recursive B^0 &&
>  
> -		test 5 -eq $(git ls-files -s | wc -l) &&
> -		test 2 -eq $(git ls-files -u b | wc -l) &&
> -		test 2 -eq $(git ls-files -u c | wc -l) &&
> -		test 4 -eq $(git ls-files -o | wc -l) &&
> +		git ls-files -s >out &&
> +		test_line_count = 5 out &&
> +		git ls-files -u b >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -u c >out &&
> +		test_line_count = 2 out &&
> +		git ls-files -o >out &&
> +		test_line_count = 5 out &&
>  
>  		test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
>  		test $(git rev-parse :2:b) = $(git rev-parse C:b) &&
> -- 
> 2.17.0.1.gda85003413
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
  2018-05-24  7:04 [PATCH 0/5] Modernize some testcases for merge-recursive corner cases Elijah Newren
@ 2018-05-24  7:04 ` Elijah Newren
  2018-05-24 10:05   ` SZEDER Gábor
  0 siblings, 1 reply; 8+ messages in thread
From: Elijah Newren @ 2018-05-24  7:04 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6036-recursive-corner-cases.sh    | 102 ++++++++++++++++++---------
 t/t6042-merge-rename-corner-cases.sh |  99 +++++++++++++++++---------
 2 files changed, 134 insertions(+), 67 deletions(-)

diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index cfe6a99771..3e659cff28 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -65,9 +65,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
 
 		test_must_fail git merge -s recursive R2^0 &&
 
-		test 2 = $(git ls-files -s | wc -l) &&
-		test 2 = $(git ls-files -u | wc -l) &&
-		test 2 = $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 3 out &&
 
 		test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
 		test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
@@ -135,9 +138,12 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
 
 		test_must_fail git merge -s recursive R2^0 &&
 
-		test 2 = $(git ls-files -s | wc -l) &&
-		test 2 = $(git ls-files -u | wc -l) &&
-		test 2 = $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 3 out &&
 
 		test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
 		test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
@@ -211,9 +217,12 @@ test_expect_success 'git detects differently handled merges conflict' '
 
 		test_must_fail git merge -s recursive E^0 &&
 
-		test 3 = $(git ls-files -s | wc -l) &&
-		test 3 = $(git ls-files -u | wc -l) &&
-		test 0 = $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 3 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse :2:new_a) = $(git rev-parse D:new_a) &&
 		test $(git rev-parse :3:new_a) = $(git rev-parse E:new_a) &&
@@ -297,8 +306,10 @@ test_expect_success 'git detects conflict merging criss-cross+modify/delete' '
 
 		test_must_fail git merge -s recursive E^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 2 -eq $(git ls-files -u | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
 
 		test $(git rev-parse :1:file) = $(git rev-parse master:file) &&
 		test $(git rev-parse :2:file) = $(git rev-parse B:file)
@@ -314,8 +325,10 @@ test_expect_success 'git detects conflict merging criss-cross+modify/delete, rev
 
 		test_must_fail git merge -s recursive D^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 2 -eq $(git ls-files -u | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
 
 		test $(git rev-parse :1:file) = $(git rev-parse master:file) &&
 		test $(git rev-parse :3:file) = $(git rev-parse B:file)
@@ -423,9 +436,12 @@ test_expect_success 'merge of D & E1 fails but has appropriate contents' '
 
 		test_must_fail git merge -s recursive E1^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 1 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 1 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
 		test $(git rev-parse :2:a) = $(git rev-parse B:a)
@@ -440,9 +456,12 @@ test_expect_success 'merge of E1 & D fails but has appropriate contents' '
 
 		test_must_fail git merge -s recursive D^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 1 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 1 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
 		test $(git rev-parse :3:a) = $(git rev-parse B:a)
@@ -457,9 +476,12 @@ test_expect_success 'merge of D & E2 fails but has appropriate contents' '
 
 		test_must_fail git merge -s recursive E2^0 &&
 
-		test 4 -eq $(git ls-files -s | wc -l) &&
-		test 3 -eq $(git ls-files -u | wc -l) &&
-		test 1 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 4 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		git ls-files -o >out &&
+		test_line_count = 2 out &&
 
 		test $(git rev-parse :2:a) = $(git rev-parse B:a) &&
 		test $(git rev-parse :3:a/file) = $(git rev-parse E2:a/file) &&
@@ -478,9 +500,12 @@ test_expect_success 'merge of E2 & D fails but has appropriate contents' '
 
 		test_must_fail git merge -s recursive D^0 &&
 
-		test 4 -eq $(git ls-files -s | wc -l) &&
-		test 3 -eq $(git ls-files -u | wc -l) &&
-		test 1 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 4 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		git ls-files -o >out &&
+		test_line_count = 2 out &&
 
 		test $(git rev-parse :3:a) = $(git rev-parse B:a) &&
 		test $(git rev-parse :2:a/file) = $(git rev-parse E2:a/file) &&
@@ -574,9 +599,12 @@ test_expect_success 'handle rename/rename(1to2)/modify followed by what looks li
 
 		git merge -s recursive E^0 &&
 
-		test 1 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 1 out &&
+		git ls-files -u >out &&
+		test_line_count = 0 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse HEAD:newname) = $(git rev-parse E:newname)
 	)
@@ -653,9 +681,12 @@ test_expect_failure 'detect rename/rename/add-source for virtual merge-base' '
 
 		git merge -s recursive E^0 &&
 
-		test 3 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 3 out &&
+		git ls-files -u >out &&
+		test_line_count = 0 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse HEAD:b) = $(git rev-parse A:a) &&
 		test $(git rev-parse HEAD:c) = $(git rev-parse A:a) &&
@@ -727,9 +758,12 @@ test_expect_success 'virtual merge base handles rename/rename(1to2)/add-dest' '
 
 		git merge -s recursive E^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 0 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse HEAD:a) = $(git rev-parse A:a) &&
 		test $(git rev-parse HEAD:c) = $(git rev-parse E:c)
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index bec0192c3b..b76da8fcdf 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -116,9 +116,12 @@ test_expect_failure 'conflict caused if rename not detected' '
 		git checkout -q C^0 &&
 		git merge -s recursive B^0 &&
 
-		test 3 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 3 out &&
+		git ls-files -u >out &&
+		test_line_count = 0 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test_line_count = 6 c &&
 		test $(git rev-parse HEAD:a) = $(git rev-parse B:a) &&
@@ -203,9 +206,12 @@ test_expect_failure 'detect rename/add-source and preserve all data' '
 
 		git merge -s recursive C^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 2 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test -f a &&
 		test -f b &&
@@ -223,9 +229,12 @@ test_expect_failure 'detect rename/add-source and preserve all data, merge other
 
 		git merge -s recursive B^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 2 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test -f a &&
 		test -f b &&
@@ -279,9 +288,12 @@ test_expect_success 'rename/directory conflict + clean content merge' '
 
 		test_must_fail git merge -s recursive right^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 1 -eq $(git ls-files -u | wc -l) &&
-		test 1 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 1 out &&
+		git ls-files -o >out &&
+		test_line_count = 2 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:file >>expect &&
@@ -307,9 +319,12 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 
 		test_must_fail git merge -s recursive right^0 &&
 
-		test 4 -eq $(git ls-files -s | wc -l) &&
-		test 3 -eq $(git ls-files -u | wc -l) &&
-		test 1 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 4 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		git ls-files -o >out &&
+		test_line_count = 2 out &&
 
 		git cat-file -p left-conflict:newfile >left &&
 		git cat-file -p base:file    >base &&
@@ -367,9 +382,12 @@ test_expect_success 'disappearing dir in rename/directory conflict handled' '
 
 		git merge -s recursive right^0 &&
 
-		test 1 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 1 out &&
+		git ls-files -u >out &&
+		test_line_count = 0 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		echo 0 >expect &&
 		git cat-file -p base:sub/file >>expect &&
@@ -426,10 +444,14 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '
 		test_must_fail git merge -s recursive C^0 >out &&
 		test_i18ngrep "CONFLICT (rename/rename)" out &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 2 -eq $(git ls-files -u | wc -l) &&
-		test 2 -eq $(git ls-files -u c | wc -l) &&
-		test 3 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -u >out &&
+		test_line_count = 2 out &&
+		git ls-files -u c >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 3 out &&
 
 		test ! -f a &&
 		test ! -f b &&
@@ -476,9 +498,12 @@ test_expect_success 'merge has correct working tree contents' '
 
 		test_must_fail git merge -s recursive B^0 &&
 
-		test 3 -eq $(git ls-files -s | wc -l) &&
-		test 3 -eq $(git ls-files -u | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 3 out &&
+		git ls-files -u >out &&
+		test_line_count = 3 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
 		test $(git rev-parse :3:b) = $(git rev-parse A:a) &&
@@ -527,8 +552,10 @@ test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge'
 
 		test_must_fail git merge -s recursive C^0 &&
 
-		test 4 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 4 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse 3:a) = $(git rev-parse C:a) &&
 		test $(git rev-parse 1:a) = $(git rev-parse A:a) &&
@@ -573,8 +600,10 @@ test_expect_failure 'rename/rename/add-source still tracks new a file' '
 		git checkout C^0 &&
 		git merge -s recursive B^0 &&
 
-		test 2 -eq $(git ls-files -s | wc -l) &&
-		test 0 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 1 out &&
 
 		test $(git rev-parse HEAD:a) = $(git rev-parse C:a) &&
 		test $(git rev-parse HEAD:b) = $(git rev-parse A:a)
@@ -615,10 +644,14 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
 		git checkout C^0 &&
 		test_must_fail git merge -s recursive B^0 &&
 
-		test 5 -eq $(git ls-files -s | wc -l) &&
-		test 2 -eq $(git ls-files -u b | wc -l) &&
-		test 2 -eq $(git ls-files -u c | wc -l) &&
-		test 4 -eq $(git ls-files -o | wc -l) &&
+		git ls-files -s >out &&
+		test_line_count = 5 out &&
+		git ls-files -u b >out &&
+		test_line_count = 2 out &&
+		git ls-files -u c >out &&
+		test_line_count = 2 out &&
+		git ls-files -o >out &&
+		test_line_count = 5 out &&
 
 		test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
 		test $(git rev-parse :2:b) = $(git rev-parse C:b) &&
-- 
2.17.0.1.gda85003413


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-05-26 23:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CABPp-BEcTKaPPUOVqTRUAW+LBBySCK0dgx1J66hYB30yMasK_Q@mail.gmail.com/>
2018-05-26  1:09 ` [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l Elijah Newren
2018-05-26 23:58   ` Junio C Hamano
2018-05-24  7:04 [PATCH 0/5] Modernize some testcases for merge-recursive corner cases Elijah Newren
2018-05-24  7:04 ` [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l Elijah Newren
2018-05-24 10:05   ` SZEDER Gábor
2018-05-24 10:36     ` SZEDER Gábor
2018-05-24 17:03     ` Elijah Newren
2018-05-25  1:17     ` Junio C Hamano
2018-05-26  0:44       ` Elijah Newren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.