| Message ID | 20250304092722.25757-1-danimahendra0904@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/1] t1403: verify that path exists and is a file | expand |
On Tue, Mar 04, 2025 at 02:57:22PM +0530, Mahendra Dani wrote: > 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" > } && There is no need to execute both functions. The underlying implementation of these functions use `test -e` and `test -f`, respectively. The former merely checks whether a path exists, whereas the latter verifies that the path is a file. It follows that when the path is a file it also has to exist, so using `test -e` (or rather its wrapper function `test_path_exists`) is redundant. Patrick
On Tue, Mar 4, 2025 at 4:33 PM Patrick Steinhardt <ps@pks.im> wrote: > > On Tue, Mar 04, 2025 at 02:57:22PM +0530, Mahendra Dani wrote: > > 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" > > } && > > There is no need to execute both functions. The underlying > implementation of these functions use `test -e` and `test -f`, > respectively. The former merely checks whether a path exists, whereas > the latter verifies that the path is a file. It follows that when the > path is a file it also has to exist, so using `test -e` (or rather its > wrapper function `test_path_exists`) is redundant. > > Patrick Sure I will remove that test_path_exists() check in the patch v4.
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(-)