# [PATCH 1/1] t5401: prefer test_path_is_* helper function 4 messages from 2025-02-01 to 2025-02-02. Participants: ambar chakravartty, Meet Soni, ambar, Junio C Hamano. Thread: https://gitlist.dev/t/62885 ## ambar chakravartty, 2025-02-01 07:12 Subject: [PATCH 1/1] t5401: prefer test_path_is_* helper function Message-ID: <20250201071210.30509-1-amch9605@gmail.com> URL: https://gitlist.dev/e/20250201071210.30509-1-amch9605%40gmail.com ``` From: ambar chakravartty test -f does not provide a nice error message when we hit test failures, so use test_path_is_file instead Signed-off-by: ambar chakravartty --- t/t5401-update-hooks.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh index 723d1e17ec..17a46fd3ba 100755 --- a/t/t5401-update-hooks.sh +++ b/t/t5401-update-hooks.sh @@ -64,14 +64,14 @@ test_expect_success 'updated as expected' ' ' test_expect_success 'hooks ran' ' - test -f victim.git/pre-receive.args && - test -f victim.git/pre-receive.stdin && - test -f victim.git/update.args && - test -f victim.git/update.stdin && - test -f victim.git/post-receive.args && - test -f victim.git/post-receive.stdin && - test -f victim.git/post-update.args && - test -f victim.git/post-update.stdin + test_path_is_file victim.git/pre-receive.args && + test_path_is_file victim.git/pre-receive.stdin && + test_path_is_file victim.git/update.args && + test_path_is_file victim.git/update.stdin && + test_path_is_file victim.git/post-receive.args && + test_path_is_file victim.git/post-receive.stdin && + test_path_is_file victim.git/post-update.args && + test_path_is_file victim.git/post-update.stdin ' test_expect_success 'pre-receive hook input' ' -- 2.48.1 ``` ## Meet Soni, 2025-02-01 09:32 Subject: Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function Message-ID: URL: https://gitlist.dev/e/CAPhwyn0tGHuX_Gh%3Drno9wj8fLb6zG4M3QAZyQDQ8qZyE%2BUyg_Q%40mail.gmail.com In-Reply-To: <20250201071210.30509-1-amch9605@gmail.com> ``` Hi Ambar On Sat, 1 Feb 2025 at 12:43, ambar chakravartty wrote: > > From: ambar chakravartty > > test -f does not provide a nice error message when we hit test > failures, so use test_path_is_file instead > > Signed-off-by: ambar chakravartty > --- Instead of sending a separate cover letter, you can add it here between "---" and diffstat. cf. https://github.com/git/git/blob/58b5801aa94ad5031978f8e42c1be1230b3d352f/Documentation/MyFirstContribution.txt#L1220 > t/t5401-update-hooks.sh | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh > index 723d1e17ec..17a46fd3ba 100755 > --- a/t/t5401-update-hooks.sh > +++ b/t/t5401-update-hooks.sh > @@ -64,14 +64,14 @@ test_expect_success 'updated as expected' ' > ' > > test_expect_success 'hooks ran' ' > - test -f victim.git/pre-receive.args && > - test -f victim.git/pre-receive.stdin && > - test -f victim.git/update.args && > - test -f victim.git/update.stdin && > - test -f victim.git/post-receive.args && > - test -f victim.git/post-receive.stdin && > - test -f victim.git/post-update.args && > - test -f victim.git/post-update.stdin > + test_path_is_file victim.git/pre-receive.args && > + test_path_is_file victim.git/pre-receive.stdin && > + test_path_is_file victim.git/update.args && > + test_path_is_file victim.git/update.stdin && > + test_path_is_file victim.git/post-receive.args && > + test_path_is_file victim.git/post-receive.stdin && > + test_path_is_file victim.git/post-update.args && > + test_path_is_file victim.git/post-update.stdin > ' > > test_expect_success 'pre-receive hook input' ' > -- > 2.48.1 > > The patch looks great! Thanks. Meet ``` ## ambar, 2025-02-01 13:29 Subject: Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function Message-ID: URL: https://gitlist.dev/e/CAMG7Y2w5ANKYG6BsfSmELtewvgU3A0hpYm35EMi__70OO_rtnw%40mail.gmail.com In-Reply-To: ``` > Instead of sending a separate cover letter, you can add it here between > "---" and diffstat. Understood. Should I resend this patch with the change ? > The patch looks great! Thanks. Thank you for your time and feedback :-) Ambar On Sat, Feb 1, 2025 at 3:02 PM Meet Soni wrote: > > Hi Ambar > On Sat, 1 Feb 2025 at 12:43, ambar chakravartty wrote: > > > > From: ambar chakravartty > > > > test -f does not provide a nice error message when we hit test > > failures, so use test_path_is_file instead > > > > Signed-off-by: ambar chakravartty > > --- > Instead of sending a separate cover letter, you can add it here between > "---" and diffstat. > cf. https://github.com/git/git/blob/58b5801aa94ad5031978f8e42c1be1230b3d352f/Documentation/MyFirstContribution.txt#L1220 > > t/t5401-update-hooks.sh | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh > > index 723d1e17ec..17a46fd3ba 100755 > > --- a/t/t5401-update-hooks.sh > > +++ b/t/t5401-update-hooks.sh > > @@ -64,14 +64,14 @@ test_expect_success 'updated as expected' ' > > ' > > > > test_expect_success 'hooks ran' ' > > - test -f victim.git/pre-receive.args && > > - test -f victim.git/pre-receive.stdin && > > - test -f victim.git/update.args && > > - test -f victim.git/update.stdin && > > - test -f victim.git/post-receive.args && > > - test -f victim.git/post-receive.stdin && > > - test -f victim.git/post-update.args && > > - test -f victim.git/post-update.stdin > > + test_path_is_file victim.git/pre-receive.args && > > + test_path_is_file victim.git/pre-receive.stdin && > > + test_path_is_file victim.git/update.args && > > + test_path_is_file victim.git/update.stdin && > > + test_path_is_file victim.git/post-receive.args && > > + test_path_is_file victim.git/post-receive.stdin && > > + test_path_is_file victim.git/post-update.args && > > + test_path_is_file victim.git/post-update.stdin > > ' > > > > test_expect_success 'pre-receive hook input' ' > > -- > > 2.48.1 > > > > > The patch looks great! Thanks. > Meet ``` ## Junio C Hamano, 2025-02-02 23:43 Subject: Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function Message-ID: URL: https://gitlist.dev/e/xmqqjza73oax.fsf%40gitster.g In-Reply-To: <20250201071210.30509-1-amch9605@gmail.com> ``` ambar chakravartty writes: > From: ambar chakravartty > > test -f does not provide a nice error message when we hit test > failures, so use test_path_is_file instead > > Signed-off-by: ambar chakravartty > --- Much better. You do not have to and you should not indent the proposed log message, and you want the full-stop (.) at the end of the word "instead" at the end of the sentence, but other than that this looks very good. Will queue after tweaking the log message. Thanks. > t/t5401-update-hooks.sh | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh > index 723d1e17ec..17a46fd3ba 100755 > --- a/t/t5401-update-hooks.sh > +++ b/t/t5401-update-hooks.sh > @@ -64,14 +64,14 @@ test_expect_success 'updated as expected' ' > ' > > test_expect_success 'hooks ran' ' > - test -f victim.git/pre-receive.args && > - test -f victim.git/pre-receive.stdin && > - test -f victim.git/update.args && > - test -f victim.git/update.stdin && > - test -f victim.git/post-receive.args && > - test -f victim.git/post-receive.stdin && > - test -f victim.git/post-update.args && > - test -f victim.git/post-update.stdin > + test_path_is_file victim.git/pre-receive.args && > + test_path_is_file victim.git/pre-receive.stdin && > + test_path_is_file victim.git/update.args && > + test_path_is_file victim.git/update.stdin && > + test_path_is_file victim.git/post-receive.args && > + test_path_is_file victim.git/post-receive.stdin && > + test_path_is_file victim.git/post-update.args && > + test_path_is_file victim.git/post-update.stdin > ' > > test_expect_success 'pre-receive hook input' ' ```