From: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Date: Mon, 05 Jan 2026 11:47:02 GMT
Subject: Re: [PATCH] [GSoC] t5403: use test_path_is_file instead of test -f
Message-ID: <CAG7UgES1AETfjyhCG2BSTrch+YgVSquJz193rDvpb3cKBfBEkg@mail.gmail.com>
In-Reply-To: <xmqqpl7ol55o.fsf@gitster.g>

> 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.
>
I understand, this does reintroduce the same debuggability issue.

> If I were in your position, I'd probably:
>
>  (1) first declare a victory with the current patch.
>
Agreed, I will keep the current patch as is.

>  (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?

I understand, I will follow up with a separate series along these lines.

Thanks,
Deveshi