threads / patch / 62753

patcht7407: use test_grep

Subject: [PATCH] t7407: use test_grep

## tl;dr

6 messages between Jan 7, 2025 and Jan 7, 2025. Diffs are folded; open one to read it.

replies: 5people: 5as markdown or json

Jeff King· Jan 7, 2025, 07:18 UTC · lore

There are a few grep calls here that can benefit from test_grep, which produces more user-friendly output when it fails.

One of these calls also passes "-sq", which is curious. The "-q" option suppresses the matched output. But test output is either already redirected to /dev/null in non-verbose mode, and in verbose mode it's better to see the output. The "-s" option suppresses errors opening files, but we are just grepping in the "expected" file we just generated, so it should not be needed. Neither of these was really hurting anything, but they are not a style we'd like to see emulated. So get rid of them.

(It is also curious to grep in the expected file in the first place, but that is because we are auto-generating the expectation from a Git command. So this is double-checking it did what we wanted).

Signed-off-by: Jeff King <peff@peff.net>
---
Just something I noticed while working on an unrelated topic.
 t/t7407-submodule-foreach.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to t/t7407-submodule-foreach.sh +2 −2
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 8d7b234beb..77b6d0040e 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -426,14 +426,14 @@ test_expect_success 'option-like arguments passed to foreach commands are not lo
 		git submodule foreach "echo be --quiet" > ../expected &&
 		git submodule foreach echo be --quiet > ../actual
 	) &&
-	grep -sq -e "--quiet" expected &&
+	test_grep -e "--quiet" expected &&
 	test_cmp expected actual
 '
 
 test_expect_success 'option-like arguments passed to foreach recurse correctly' '
 	git -C clone2 submodule foreach --recursive "echo be --an-option" >expect &&
 	git -C clone2 submodule foreach --recursive echo be --an-option >actual &&
-	grep -e "--an-option" expect &&
+	test_grep -e "--an-option" expect &&
 	test_cmp expect actual
 '
 
-- 
2.48.0.rc2.377.g0507c08f68
Patrick Steinhardt· Jan 7, 2025, 09:57 UTC · re: Jeff King · lore

Re: [PATCH] t7407: use test_grep

On Tue, Jan 07, 2025 at 02:18:24AM -0500, Jeff King wrote:
Show 17 quoted lines
> There are a few grep calls here that can benefit from test_grep, which
> produces more user-friendly output when it fails.
> 
> One of these calls also passes "-sq", which is curious. The "-q" option
> suppresses the matched output. But test output is either already
> redirected to /dev/null in non-verbose mode, and in verbose mode it's
> better to see the output. The "-s" option suppresses errors opening
> files, but we are just grepping in the "expected" file we just
> generated, so it should not be needed. Neither of these was really
> hurting anything, but they are not a style we'd like to see emulated. So
> get rid of them.
> 
> (It is also curious to grep in the expected file in the first place, but
> that is because we are auto-generating the expectation from a Git
> command. So this is double-checking it did what we wanted).
> 
> Signed-off-by: Jeff King <peff@peff.net>
Thanks, both of these look obviously good to me.
Patrick
Kristoffer Haugsbakk· Jan 7, 2025, 11:01 UTC · re: Jeff King · lore

Re: [PATCH] t7407: use test_grep

On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:
Show 5 quoted lines
> get rid of them.
>
> (It is also curious to grep in the expected file in the first place, but
> that is because we are auto-generating the expectation from a Git
> command. So this is double-checking it did what we wanted).
Missing “that”? s/it did/that it did/
Junio C Hamano· Jan 7, 2025, 19:42 UTC · re: Kristoffer Haugsbakk · lore

Re: [PATCH] t7407: use test_grep

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
Show 8 quoted lines
> On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:
>> get rid of them.
>>
>> (It is also curious to grep in the expected file in the first place, but
>> that is because we are auto-generating the expectation from a Git
>> command. So this is double-checking it did what we wanted).
>
> Missing “that”? s/it did/that it did/
It is colloquially correct, isn't it?
Kristoffer Haugsbakk· Jan 7, 2025, 22:09 UTC · re: Junio C Hamano · lore

Re: [PATCH] t7407: use test_grep

On Tue, Jan 7, 2025, at 20:42, Junio C Hamano wrote:
Show 12 quoted lines
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>
>> On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:
>>> get rid of them.
>>>
>>> (It is also curious to grep in the expected file in the first place, but
>>> that is because we are auto-generating the expectation from a Git
>>> command. So this is double-checking it did what we wanted).
>>
>> Missing “that”? s/it did/that it did/
>
> It is colloquially correct, isn't it?

I’m just going by intuition but I couldn’t make it make sense in my head/read out loud.

-- 
Kristoffer Haugsbakk
D. Ben Knoble· Jan 7, 2025, 23:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] t7407: use test_grep

On Tue, Jan 7, 2025 at 2:42 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
>
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>
> > On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:
> >> get rid of them.
> >>
> >> (It is also curious to grep in the expected file in the first place, but
> >> that is because we are auto-generating the expectation from a Git
> >> command. So this is double-checking it did what we wanted).
> >
> > Missing “that”? s/it did/that it did/
>
> It is colloquially correct, isn't it?
>
>
Reads fine to me.
-- 
D. Ben Knoble

← back to recent threads