threads / patch / 62885

patcht5401: prefer test_path_is_* helper function

Subject: [PATCH 1/1] t5401: prefer test_path_is_* helper function

## tl;dr

4 messages between Feb 1, 2025 and Feb 2, 2025. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

ambar chakravartty· Feb 1, 2025, 07:12 UTC · lore
From: ambar chakravartty <chakravarttyambar@gmail.com>
    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 <amch9605@gmail.com>
---
 t/t5401-update-hooks.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
Show changes to t/t5401-update-hooks.sh +8 −8
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· Feb 1, 2025, 09:32 UTC · re: ambar chakravartty · lore

Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function

Hi Ambar On Sat, 1 Feb 2025 at 12:43, ambar chakravartty <amch9605@gmail.com> wrote:

Show 8 quoted lines
>
> From: ambar chakravartty <chakravarttyambar@gmail.com>
>
>     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 <amch9605@gmail.com>
> ---

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

Show 34 quoted lines
>  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· Feb 1, 2025, 13:29 UTC · re: Meet Soni · lore

Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function

> 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 <meetsoni3017@gmail.com> wrote:
Show 50 quoted lines
>
> Hi Ambar
> On Sat, 1 Feb 2025 at 12:43, ambar chakravartty <amch9605@gmail.com> wrote:
> >
> > From: ambar chakravartty <chakravarttyambar@gmail.com>
> >
> >     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 <amch9605@gmail.com>
> > ---
> 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· Feb 2, 2025, 23:43 UTC · re: ambar chakravartty · lore

Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function

ambar chakravartty <amch9605@gmail.com> writes:
Show 7 quoted lines
> From: ambar chakravartty <chakravarttyambar@gmail.com>
>
>     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 <amch9605@gmail.com>
> ---

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.
Show 30 quoted lines
>  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' '

← back to recent threads