From: Junio C Hamano Date: Mon, 05 Jan 2026 10:34:11 GMT Subject: Re: [PATCH] [GSoC] t5403: use test_path_is_file instead of test -f Message-ID: In-Reply-To: Deveshi Dwivedi writes: > I can update the clone test to follow the same pattern as the other tests: > read old new flag 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. If I were in your position, I'd probably: (1) first declare a victory with the current patch. (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?