* 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
* [PATCH 0/5] Modernize some testcases for merge-recursive corner cases @ 2018-05-24 7:04 Elijah Newren 2018-05-24 7:04 ` [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l Elijah Newren 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 I have about a dozen more testcases I want to add to the testsuite, in order to document additional situations that merge-recursive handles suboptimally. To avoid sending an excessively long series, I am splitting this up. This series just aims to modernize a pair of testfiles in which several of the new tests would most naturally belong. This modernization serves two purposes: (1) it makes the existing tests easier to understand, and (2) it simplifies adding more tests to these test files. Elijah Newren (5): t6036, t6042: use test_create_repo to keep tests independent t6036, t6042: use test_line_count instead of wc -l t6036, t6042: prefer test_path_is_file, test_path_is_missing t6036, t6042: prefer test_cmp to sequences of test t6036: prefer test_when_finished to manual cleanup in following test t/t6036-recursive-corner-cases.sh | 991 +++++++++++++++------------ t/t6042-merge-rename-corner-cases.sh | 968 ++++++++++++++------------ 2 files changed, 1098 insertions(+), 861 deletions(-) -- 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
* 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
* 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 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 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-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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).