Re: [PATCH v3 2/4] t6402: use find(1) builtin to filter instead of grep

From: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Date: Jun 21, 2021, 23:54 UTC
Message-ID: <YNEms6638tIKnBDC@danh.dev>
In-Reply-To: <d4082001-6c70-ae02-c448-e038923c840e@gmail.com>

On 2021-06-21 10:17:52+0200, Andrei Rybak <rybak.a.v@gmail.com> wrote:

Show 9 quoted lines

> On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:
> > find(1) has a builtin (-prune) to filter its output, save a bit of time
> > for invoking grep(1).
> > 
> > In addition, in a later change, we will try to use test_line_count_cmd
> > to count number of lines in stdout and/or stderr of a command, due to
> 
> Looking at [PATCH v3 1/4] of this series, mention of "stderr" here is no
> longer relevant.

Yes, you're correct.

> > limitation of current implementation, it can handle pipe.
> 
> Seems like a typo s/can/can't/ ?

This is correct, too.

Show 33 quoted lines

> > Let's replace grep(1)'s usage with find(1) builtin filter.
> > 
> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> > ---
> >   t/t6402-merge-rename.sh | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
> > index 425dad97d5..5d76cd6414 100755
> > --- a/t/t6402-merge-rename.sh
> > +++ b/t/t6402-merge-rename.sh
> > @@ -546,7 +546,7 @@ then
> >   		test_must_fail git diff --quiet &&
> > -		test 3 -eq $(find . | grep -v .git | wc -l) &&
> > +		test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
> >   		test_path_is_file one &&
> >   		test_path_is_file two &&
> > @@ -565,7 +565,7 @@ else
> >   		test_must_fail git diff --quiet &&
> > -		test 4 -eq $(find . | grep -v .git | wc -l) &&
> > +		test 4 -eq $(find . -name .git -prune -o -print | wc -l) &&
> >   		test_path_is_dir one &&
> >   		test_path_is_file one~rename-two &&
> > @@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
> >   	test_must_fail git diff --quiet &&
> > -	test 3 -eq $(find . | grep -v .git | wc -l) &&
> > +	test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
> 
> Because in the original `grep` wasn't invoked with `-F` it means that
> ".git" is a regex which would match any path which contains the word
> "git" in it, because "." matches any character, including the leading
> slash that `find` outputs.  Such narrowing of what we intend to filter
> out is a good change.

I think the original intention is using "grep -F". I'll add that information into the commit message.

> This semantic change in filtering doesn't affect tests in t6402, as the
> test directory doesn't have paths with the word "git" except for the
> ".git" directory.  It might be worth mentioning in the commit message.

Thanks.

-- 
Danh