patch[GSoC] t5403: use test_path_is_file instead of test -f

> Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
> ---
>  t/t5403-post-checkout-hook.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 978f240cda..1462e3365b 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -109,7 +109,7 @@ test_expect_success 'post-checkout hook is triggered by clone' '
>  	echo "$@" >"$GIT_DIR/post-checkout.args"
>  	EOF
>  	git clone --template=templates . clone3 &&
> -	test -f clone3/.git/post-checkout.args
> +	test_path_is_file clone3/.git/post-checkout.args
>  '
>  
>  test_done

> > Replace 'test -f' with the test_path_is_file in
> > t5403-post-checkout-hook.sh. This helper provides better error
> > messages when tests fail, making it easier to debug issues.
>
> All true, so I'll queue the patch.  Thanks.
>
> A #leftoverbit is to think about what this test checks, if it
> makes sense, and if we can do better.  The expected outcome of this
> clone is stable, so the input fed to the hook should also be stable.
> With the same brain-cycle to write a test that checks the existence
> of the output file (i.e., proving that the hook was run), we should
> be able to concoct a test that validates the contents of the output.
>

> > Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
> > ---
> >  t/t5403-post-checkout-hook.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> > index 978f240cda..1462e3365b 100755
> > --- a/t/t5403-post-checkout-hook.sh
> > +++ b/t/t5403-post-checkout-hook.sh
> > @@ -109,7 +109,7 @@ test_expect_success 'post-checkout hook is triggered by clone' '
> >       echo "$@" >"$GIT_DIR/post-checkout.args"
> >       EOF
> >       git clone --template=templates . clone3 &&
> > -     test -f clone3/.git/post-checkout.args
> > +     test_path_is_file clone3/.git/post-checkout.args
> >  '
> >
> >  test_done

> I can update the clone test to follow the same pattern as the other tests:
> read old new flag <clone3/.git/post-checkout.args &&
> test "$old" = $(test_oid zero) &&
> test "$new" = $(git rev-parse HEAD) &&
> test "$flag" = 1
>
> Does this sound reasonable?

> The open-coded four command sequence above is repeatedly used
> throughout this test script.  I find them quite ugly but more
> importantly, they have exactly the same downside as your patch is
> trying to correct---it is almost impossible to tell where the test
> failed and how from its output, because these "test" will simply
> fail silently.
>

>  (2) as a separate series, on top of (1), prepare a patch that
>      replaces these "read old new flag, then check $old, $new, and
>      $flag" sequence with a helper function that can be called like
>      so:
>
>         check_post_checkout clone3/.git/post-checkout.args \
>                 "$(test_oid zero)" "$(git rev-parse HEAD"  1
>
>      Leave the implementation of check_post_checkout just like the
>      original, i.e., "read old new flag, and then test these three
>      things, failing silently".  The point of this step is not about
>      improving the tests; the point is to make it easier to improve
>      in the next step, without changing what the tests do.
>
>  (3) then update the implementation of check_post_checkout, with the
>      implementation of the post-checkout hook also updated to match,
>      so that the helper now looks like this:
>
>         check_post_checkout () {
>                 test "$#" = 4 || BUG "check_post_checkout takes 4 args"
>                 echo "old=$2 new=$3 flag=$4" >expect &&
>                 test_cmp expect "$1"
>         }
>
> Hmm?