git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] t5401: prefer test_path_is_* helper function
@ 2025-02-01  7:12 ambar chakravartty
  2025-02-01  9:32 ` Meet Soni
  2025-02-02 23:43 ` Junio C Hamano
  0 siblings, 2 replies; 4+ messages in thread
From: ambar chakravartty @ 2025-02-01  7:12 UTC (permalink / raw)
  To: git; +Cc: ambar chakravartty, ambar chakravartty

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

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function
  2025-02-01  7:12 [PATCH 1/1] t5401: prefer test_path_is_* helper function ambar chakravartty
@ 2025-02-01  9:32 ` Meet Soni
  2025-02-01 13:29   ` ambar
  2025-02-02 23:43 ` Junio C Hamano
  1 sibling, 1 reply; 4+ messages in thread
From: Meet Soni @ 2025-02-01  9:32 UTC (permalink / raw)
  To: ambar chakravartty; +Cc: git, ambar chakravartty

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function
  2025-02-01  9:32 ` Meet Soni
@ 2025-02-01 13:29   ` ambar
  0 siblings, 0 replies; 4+ messages in thread
From: ambar @ 2025-02-01 13:29 UTC (permalink / raw)
  To: Meet Soni; +Cc: git

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] t5401: prefer test_path_is_* helper function
  2025-02-01  7:12 [PATCH 1/1] t5401: prefer test_path_is_* helper function ambar chakravartty
  2025-02-01  9:32 ` Meet Soni
@ 2025-02-02 23:43 ` Junio C Hamano
  1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2025-02-02 23:43 UTC (permalink / raw)
  To: ambar chakravartty; +Cc: git, ambar chakravartty

ambar chakravartty <amch9605@gmail.com> writes:

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


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-02-02 23:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-01  7:12 [PATCH 1/1] t5401: prefer test_path_is_* helper function ambar chakravartty
2025-02-01  9:32 ` Meet Soni
2025-02-01 13:29   ` ambar
2025-02-02 23:43 ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).