diff mbox series

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

Message ID 20210619013035.26313-3-congdanhqx@gmail.com (mailing list archive)
State New
Headers show
Series None | expand

Commit Message

Đoàn Trần Công Danh June 19, 2021, 1:30 a.m. UTC
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
limitation of current implementation, it can handle pipe.

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

Comments

Andrei Rybak June 21, 2021, 8:17 a.m. UTC | #1
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.

> limitation of current implementation, it can handle pipe.

Seems like a typo s/can/can't/ ?

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

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.

>   
>   	test_path_is_file one &&
>   	test_path_is_file two &&
>
Đoàn Trần Công Danh June 21, 2021, 11:54 p.m. UTC | #2
On 2021-06-21 10:17:52+0200, Andrei Rybak <rybak.a.v@gmail.com> wrote:
> 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.

> > 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.
diff mbox series

Patch

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) &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&