Re: [PATCH 1/1] t1403: prefer test_path_exists helper function
- From
Mahendra Dani <danimahendra0904@gmail.com>
- Date
- Mar 4, 2025, 02:27 UTC
- Message-ID
- <CABGrwBB1-UiiPEOptN9csVP1WYg2X=SWeMxxiyDfz02VmDp1vA@mail.gmail.com>
- In-Reply-To
- <Z8WD2MeHpOH7Ni8A@pks.im>
On Mon, Mar 3, 2025 at 3:56 PM Patrick Steinhardt <ps@pks.im> wrote:
Show 32 quoted lines
>
> On Sat, Mar 01, 2025 at 04:28:38PM +0530, Mahendra Dani wrote:
> > test -e does not provide a nice error message when
> > we hit test failures, so use test_path_exists instead.
> >
> > Signed-off-by: Mahendra Dani <danimahendra0904@gmail.com>
> > ---
> > t/t1403-show-ref.sh | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
> > index 9d698b3cc3..12f7b60024 100755
> > --- a/t/t1403-show-ref.sh
> > +++ b/t/t1403-show-ref.sh
> > @@ -196,7 +196,7 @@ test_expect_success 'show-ref --verify with dangling ref' '
> >
> > remove_object() {
> > file=$(sha1_file "$*") &&
> > - test -e "$file" &&
> > + test_path_exists "$file" &&
> > rm -f "$file"
> > } &&
>
> The refactoring is true to the original spirit of the preimage indeed.
> But we could also improve it even further if we verified that the path
> not only exists, but exists and is a file via `test_path_is_file()`. If
> we decide to do that we should also explain the change in the commit
> message.
>
> Thanks!
>
> PatrickYes, sure. I will improve it further using the `test_path_is_file()` helper function and change the commit message in v2 patch.
Thanks, Mahendra.