{"thread":{"id":"62753","subject":"[PATCH] t7407: use test_grep","startedAt":"2025-01-07T07:18:25Z","lastAt":"2025-01-07T23:36:43Z","messageCount":6,"participants":["Jeff King","Patrick Steinhardt","Kristoffer Haugsbakk","Junio C Hamano","D. Ben Knoble"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"510050","messageId":"20250107071824.GA594237@coredump.intra.peff.net","threadId":"62753","inReplyTo":null,"subject":"[PATCH] t7407: use test_grep","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-07T07:18:24Z","receivedAt":"2025-01-07T07:18:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are a few grep calls here that can benefit from test_grep, which\nproduces more user-friendly output when it fails.\n\nOne of these calls also passes \"-sq\", which is curious. The \"-q\" option\nsuppresses the matched output. But test output is either already\nredirected to /dev/null in non-verbose mode, and in verbose mode it's\nbetter to see the output. The \"-s\" option suppresses errors opening\nfiles, but we are just grepping in the \"expected\" file we just\ngenerated, so it should not be needed. Neither of these was really\nhurting anything, but they are not a style we'd like to see emulated. So\nget rid of them.\n\n(It is also curious to grep in the expected file in the first place, but\nthat is because we are auto-generating the expectation from a Git\ncommand. So this is double-checking it did what we wanted).\n\nSigned-off-by: Jeff King \n---\nJust something I noticed while working on an unrelated topic.\n\n t/t7407-submodule-foreach.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 8d7b234beb..77b6d0040e 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -426,14 +426,14 @@ test_expect_success 'option-like arguments passed to foreach commands are not lo\n \t\tgit submodule foreach \"echo be --quiet\" > ../expected &&\n \t\tgit submodule foreach echo be --quiet > ../actual\n \t) &&\n-\tgrep -sq -e \"--quiet\" expected &&\n+\ttest_grep -e \"--quiet\" expected &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'option-like arguments passed to foreach recurse correctly' '\n \tgit -C clone2 submodule foreach --recursive \"echo be --an-option\" >expect &&\n \tgit -C clone2 submodule foreach --recursive echo be --an-option >actual &&\n-\tgrep -e \"--an-option\" expect &&\n+\ttest_grep -e \"--an-option\" expect &&\n \ttest_cmp expect actual\n '\n \n-- \n2.48.0.rc2.377.g0507c08f68\n"},{"id":"510060","messageId":"Z3z6dKBRjQlsrwjZ@pks.im","threadId":"62753","inReplyTo":"20250107071824.GA594237@coredump.intra.peff.net","subject":"Re: [PATCH] t7407: use test_grep","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-07T09:57:08Z","receivedAt":"2025-01-07T09:57:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jan 07, 2025 at 02:18:24AM -0500, Jeff King wrote:\n> There are a few grep calls here that can benefit from test_grep, which\n> produces more user-friendly output when it fails.\n> \n> One of these calls also passes \"-sq\", which is curious. The \"-q\" option\n> suppresses the matched output. But test output is either already\n> redirected to /dev/null in non-verbose mode, and in verbose mode it's\n> better to see the output. The \"-s\" option suppresses errors opening\n> files, but we are just grepping in the \"expected\" file we just\n> generated, so it should not be needed. Neither of these was really\n> hurting anything, but they are not a style we'd like to see emulated. So\n> get rid of them.\n> \n> (It is also curious to grep in the expected file in the first place, but\n> that is because we are auto-generating the expectation from a Git\n> command. So this is double-checking it did what we wanted).\n> \n> Signed-off-by: Jeff King \n\nThanks, both of these look obviously good to me.\n\nPatrick\n"},{"id":"510061","messageId":"f3ab08db-5dff-4da3-a075-7e9b3eaf1f78@app.fastmail.com","threadId":"62753","inReplyTo":"20250107071824.GA594237@coredump.intra.peff.net","subject":"Re: [PATCH] t7407: use test_grep","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-01-07T11:01:14Z","receivedAt":"2025-01-07T11:01:39Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:\n> get rid of them.\n>\n> (It is also curious to grep in the expected file in the first place, but\n> that is because we are auto-generating the expectation from a Git\n> command. So this is double-checking it did what we wanted).\n\nMissing “that”? s/it did/that it did/\n"},{"id":"510111","messageId":"xmqq5xmqs95j.fsf@gitster.g","threadId":"62753","inReplyTo":"f3ab08db-5dff-4da3-a075-7e9b3eaf1f78@app.fastmail.com","subject":"Re: [PATCH] t7407: use test_grep","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-07T19:42:32Z","receivedAt":"2025-01-07T19:42:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:\n>> get rid of them.\n>>\n>> (It is also curious to grep in the expected file in the first place, but\n>> that is because we are auto-generating the expectation from a Git\n>> command. So this is double-checking it did what we wanted).\n>\n> Missing “that”? s/it did/that it did/\n\nIt is colloquially correct, isn't it?\n\n"},{"id":"510128","messageId":"8aa4e7d9-b77b-4f29-9564-5c1d4ec5be0a@app.fastmail.com","threadId":"62753","inReplyTo":"xmqq5xmqs95j.fsf@gitster.g","subject":"Re: [PATCH] t7407: use test_grep","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-01-07T22:09:33Z","receivedAt":"2025-01-07T22:09:55Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, Jan 7, 2025, at 20:42, Junio C Hamano wrote:\n> \"Kristoffer Haugsbakk\" writes:\n>\n>> On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:\n>>> get rid of them.\n>>>\n>>> (It is also curious to grep in the expected file in the first place, but\n>>> that is because we are auto-generating the expectation from a Git\n>>> command. So this is double-checking it did what we wanted).\n>>\n>> Missing “that”? s/it did/that it did/\n>\n> It is colloquially correct, isn't it?\n\nI’m just going by intuition but I couldn’t make it make\nsense in my head/read out loud.\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"510134","messageId":"CALnO6CAUZBB28vCjMF8HRxvdv=N6y6OVyXZF2Yh4rK8c-sQU1A@mail.gmail.com","threadId":"62753","inReplyTo":"xmqq5xmqs95j.fsf@gitster.g","subject":"Re: [PATCH] t7407: use test_grep","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-01-07T23:36:26Z","receivedAt":"2025-01-07T23:36:43Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Tue, Jan 7, 2025 at 2:42 PM Junio C Hamano wrote:\n>\n> \"Kristoffer Haugsbakk\" writes:\n>\n> > On Tue, Jan 7, 2025, at 08:18, Jeff King wrote:\n> >> get rid of them.\n> >>\n> >> (It is also curious to grep in the expected file in the first place, but\n> >> that is because we are auto-generating the expectation from a Git\n> >> command. So this is double-checking it did what we wanted).\n> >\n> > Missing “that”? s/it did/that it did/\n>\n> It is colloquially correct, isn't it?\n>\n>\n\nReads fine to me.\n\n\n-- \nD. Ben Knoble\n"}]}