| Message ID | 20250304094153.28959-2-danimahendra0904@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | t1403: verify path exists and is a file | expand |
Mahendra Dani <danimahendra0904@gmail.com> writes: > test -e does not provide a nice error message when > we hit test failures, so use test_path_exists() instead > and verify that if the path exists then it is a file using test_path_is_file(). > > Signed-off-by: Mahendra Dani <danimahendra0904@gmail.com> > --- > t/t1403-show-ref.sh | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh > index 9d698b3cc3..4afde01a29 100755 > --- a/t/t1403-show-ref.sh > +++ b/t/t1403-show-ref.sh > @@ -196,7 +196,8 @@ test_expect_success 'show-ref --verify with dangling ref' ' > > remove_object() { > file=$(sha1_file "$*") && > - test -e "$file" && > + test_path_exists "$file" && > + test_path_is_file "$file" && > rm -f "$file" > } && Makes sense. Will queue.
Junio C Hamano <gitster@pobox.com> writes: > Mahendra Dani <danimahendra0904@gmail.com> writes: > >> test -e does not provide a nice error message when >> we hit test failures, so use test_path_exists() instead >> and verify that if the path exists then it is a file using test_path_is_file(). >> >> Signed-off-by: Mahendra Dani <danimahendra0904@gmail.com> >> --- >> t/t1403-show-ref.sh | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh >> index 9d698b3cc3..4afde01a29 100755 >> --- a/t/t1403-show-ref.sh >> +++ b/t/t1403-show-ref.sh >> @@ -196,7 +196,8 @@ test_expect_success 'show-ref --verify with dangling ref' ' >> >> remove_object() { >> file=$(sha1_file "$*") && >> - test -e "$file" && >> + test_path_exists "$file" && >> + test_path_is_file "$file" && >> rm -f "$file" >> } && > > Makes sense. Will queue. No, no, no. test_is_file alone is sufficient---if the thing does not exist, it would not be a file anyway ;-)
On Tue, Mar 4, 2025 at 11:36 PM Junio C Hamano <gitster@pobox.com> wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > > Mahendra Dani <danimahendra0904@gmail.com> writes: > > > >> test -e does not provide a nice error message when > >> we hit test failures, so use test_path_exists() instead > >> and verify that if the path exists then it is a file using test_path_is_file(). > >> > >> Signed-off-by: Mahendra Dani <danimahendra0904@gmail.com> > >> --- > >> t/t1403-show-ref.sh | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh > >> index 9d698b3cc3..4afde01a29 100755 > >> --- a/t/t1403-show-ref.sh > >> +++ b/t/t1403-show-ref.sh > >> @@ -196,7 +196,8 @@ test_expect_success 'show-ref --verify with dangling ref' ' > >> > >> remove_object() { > >> file=$(sha1_file "$*") && > >> - test -e "$file" && > >> + test_path_exists "$file" && > >> + test_path_is_file "$file" && > >> rm -f "$file" > >> } && > > > > Makes sense. Will queue. > > No, no, no. test_is_file alone is sufficient---if the thing does > not exist, it would not be a file anyway ;-) > Yes, it was pointed out by Patrick in [1], that `test_path_is_file()` alone is sufficient. Hence I removed the `test_path_exists()` check in patch v4 [2]. [References] 1. https://lore.kernel.org/git/Z8bd3iHrhXb4WH6A@pks.im/ 2. https://lore.kernel.org/git/20250304112728.41228-2-danimahendra0904@gmail.com/
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh index 9d698b3cc3..4afde01a29 100755 --- a/t/t1403-show-ref.sh +++ b/t/t1403-show-ref.sh @@ -196,7 +196,8 @@ test_expect_success 'show-ref --verify with dangling ref' ' remove_object() { file=$(sha1_file "$*") && - test -e "$file" && + test_path_exists "$file" && + test_path_is_file "$file" && rm -f "$file" } &&
test -e does not provide a nice error message when we hit test failures, so use test_path_exists() instead and verify that if the path exists then it is a file using test_path_is_file(). Signed-off-by: Mahendra Dani <danimahendra0904@gmail.com> --- t/t1403-show-ref.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)