Re: [PATCH] t0410: modernize delete_object helper
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 12, 2026, 19:05 UTC
- Message-ID
- <CAPig+cS3v=OT6BJ0WWh=qvWBm1TVck+O7eKd7gJ2fe_d5Rny_A@mail.gmail.com>
- In-Reply-To
- <20260312125030.7799-1-r.siddharth.shrimali@gmail.com>
On Thu, Mar 12, 2026 at 8:50 AM Siddharth Shrimali <r.siddharth.shrimali@gmail.com> wrote:
Show 24 quoted lines
> The delete_object helper currently relies on a manual sed command to
> calculate object paths. This works, but it's a bit brittle and forces
> us to maintain shell logic that Git's own test suite can already
> handle more elegantly.
>
> Switch to 'test_oid_to_path' to let Git handle the path logic. This
> makes the helper hash independent, which is much cleaner than manual
> string manipulation. While we're at it, add a call to
> 'test_path_is_file' so that the test fails early and clearly if we
> try to delete an object that isn't there, rather than failing
> silently.
>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
> @@ -11,7 +11,11 @@ test_description='partial clone'
> delete_object () {
> - rm $1/.git/objects/$(echo $2 | sed -e 's|^..|&/|')
> + repo=$1
> + obj=$2
> + path="$repo/.git/objects/$(test_oid_to_path $obj)" &&
> + test_path_is_file "$path" &&
> + rm "$path"
> }Despite what the commit message says, adding a call to `test_path_is_file` here does not add value since `rm` will already fail noisily and exit with an error code if the path does not exist. Moreover, because it's unnecessary, the `test_path_is_file` invocation may confuse readers into thinking that something subtle is going on that requires extra scrutiny and care even though that's not the case. So let's not add this needless extra code.