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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2623 bytes --]

On Tue, Jan 23, 2024 at 05:15:56PM +0100, Christian Couder wrote:
> 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.
> 
> > 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.

They all fail in the same way: we fail to access the repository because
we delete the extension that tells us to use the reftable backend. I'll
add that explanation at the front.

> > 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.

Good catch.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]