Re: [PATCH v2 3/6] t1302: make tests more robust with new extensions

From: Christian Couder <christian.couder@gmail.com>
Date: Jan 23, 2024, 16:15 UTC
Message-ID: <CAP8UFD0yh4k4p7zcM+ZF90oWWTUMoGVpd3ajWWOV5pk++YKerw@mail.gmail.com>
In-Reply-To: <1284b70fab6dd85114cb24fc5c7b6c49e35eb135.1704877233.git.ps@pks.im>

On Wed, Jan 10, 2024 at 10:02 AM Patrick Steinhardt <ps@pks.im> wrote:

>
> In t1302 we exercise logic around "core.repositoryFormatVersion" and
> extensions. These tests are not particularly robust against extensions
> like the newly introduced "refStorage" extension.

Here also it's not very clear what robust means. Some examples of how things could fail would perhaps help.

Show 12 quoted lines

> Refactor the tests to be more robust:
>
>   - Check the DEFAULT_REPO_FORMAT prereq to determine the expected
>     repository format version. This helps to ensure that we only need to
>     update the prereq in a central place when new extensions are added.
>
>   - Use a separate repository to rewrite ".git/config" to test
>     combinations of the repository format version and extensions. This
>     ensures that we don't break the main test repository.
>
>   - Do not rewrite ".git/config" when exercising the "preciousObjects"
>     extension.

It would be nice to talk a bit about the failures that each of the above changes could prevent.

Show 24 quoted lines

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t1302-repo-version.sh | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
> index 179474fa65..7c680c91c4 100755
> --- a/t/t1302-repo-version.sh
> +++ b/t/t1302-repo-version.sh
> @@ -28,7 +28,12 @@ test_expect_success 'setup' '
>  '
>
>  test_expect_success 'gitdir selection on normal repos' '
> -       test_oid version >expect &&
> +       if test_have_prereq DEFAULT_REPO_FORMAT
> +       then
> +               echo 0
> +       else
> +               echo 1
> +       fi >expect &&
>         git config core.repositoryformatversion >actual &&
>         git -C test config core.repositoryformatversion >actual2 &&
>         test_cmp expect actual &&
> @@ -79,8 +84,13 @@ mkconfig () {

Before that hunk there is:

test_expect_success 'setup' '
    test_oid_cache <<-\EOF &&
    version sha1:0
    version sha256:1
    EOF

and it seems to me that the above test_oid_cache call is not needed anymore, if `test_oid version` is not used anymore.

Otherwise this looks good to me.