| Message ID | 20250201071210.30509-1-amch9605@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 318f4c98276de7e515f838a9626bcf60d757ee20 |
| Headers | show |
| Series | t5401: prefer test_path_is_* helper function | expand |
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
> 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
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' '
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' '