From: Eric Sunshine <sunshine@sunshineco.com>
Date: Wed, 19 Nov 2025 22:13:19 GMT
Subject: Re: [PATCH 2/2] refs: add GIT_REF_URI to specify reference backend and directory
Message-ID: <CAPig+cQ1nF47Rvvm4gDks8Bzj0htzUkfR24WB8UdnJtcQTQwCA@mail.gmail.com>
In-Reply-To: <20251119-kn-alternate-ref-dir-v1-2-4cf4a94c8bed@gmail.com>

On Wed, Nov 19, 2025 at 4:49 PM Karthik Nayak <karthik.188@gmail.com> wrote:
> Git allows setting a different object directory via
> 'GIT_OBJECT_DIRECTORY', but provides no equivalent for references.
> This asymmetry makes it difficult to test different reference backends
> or use alternative reference storage locations without modifying the
> repository structure.
>
> Add a new environment variable 'GIT_REF_URI' that specifies both the
> reference backend and directory path using a URI format:
> [...]
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/t/t1423-ref-backend.sh b/t/t1423-ref-backend.sh
> @@ -0,0 +1,109 @@
> +test_expect_success 'empty uri provided' '
> +       test_when_finished "rm -rf repo" &&
> +       git init --ref-format=files repo &&
> +       (
> +               cd repo &&
> +               GIT_REF_URI="" &&
> +               export GIT_REF_URI &&
> +               ! git refs list 2>err &&

Should this (and all other tests) be using `test_must_fail` rather than `!`?

> +               test_grep "reference backend uri is empty" err
> +       )
> +'
> +ref_formats="files reftable"
> +for from_format in $ref_formats
> +do
> +       for to_format in $ref_formats
> +       do
> +               if test "$from_format" = "$to_format"
> +               then
> +                       continue
> +               fi
> +
> +               test_expect_success 'read from other reference backend' '
> +                       test_when_finished "rm -rf repo" &&
> +                       git init --ref-format=files repo &&
> +                       (
> +                               cd repo &&
> +                               test_commit 1 &&
> +                               test_commit 2 &&
> +                               test_commit 3 &&
> +
> +                               git refs migrate --dry-run --ref-format=reftable >out &&
> +                               REFTABLE_PATH=$(cat out | sed "s/.* ${SQ}\(.*\)${SQ}/\1/") &&
> +                               git refs list >expect &&
> +                               GIT_REF_URI="reftable://$REFTABLE_PATH" git refs list >actual &&
> +                               test_cmp expect actual
> +                       )
> +               '
> +
> +               test_expect_success 'write to other reference backend' '
> +                       [...]
> +               '
> +       done
> +done

Something seems amiss here. Presumably, this nested loop wants to test
various combinations but the `from_format` and `to_format` variables
are never consulted in the tests; instead the tests just hardcode
specific ref-format values.

Also, if this is indeed meant to be loop-driven, then it would be
helpful for the test titles to include the values of `$from_format`
and `$to_format`.