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@xxxxxx> 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@xxxxxx>
> > ---
> > 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:
signature.asc
Description: PGP signature
