git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l
Date: Thu, 24 May 2018 12:05:07 +0200	[thread overview]
Message-ID: <20180524100507.32305-1-szeder.dev@gmail.com> (raw)
In-Reply-To: <20180524070439.6367-3-newren@gmail.com>


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

  reply	other threads:[~2018-05-24 10:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-24  7:04 [PATCH 0/5] Modernize some testcases for merge-recursive corner cases Elijah Newren
2018-05-24  7:04 ` [PATCH 1/5] t6036, t6042: use test_create_repo to keep tests independent 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 [this message]
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
2018-05-24  7:04 ` [PATCH 3/5] t6036, t6042: prefer test_path_is_file, test_path_is_missing Elijah Newren
2018-05-24  7:04 ` [PATCH 4/5] t6036, t6042: prefer test_cmp to sequences of test Elijah Newren
2018-05-24  7:04 ` [PATCH 5/5] t6036: prefer test_when_finished to manual cleanup in following test Elijah Newren
     [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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180524100507.32305-1-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).