On Tue, Apr 11, 2023 at 09:51:06AM -0400, Derrick Stolee wrote:
> On 4/10/2023 6:53 PM, Taylor Blau wrote:
> > Instead of getting rid of the option, invert its meaning to instead
> > disable writing ".rev" files, thereby running the test suite in a mode
> > where the reverse index is generated from scratch.
> >
> > This ensures that we are still running and exercising Git's behavior
> > when forced to generate reverse indexes from scratch.
>
> I don't think this is true because you remove the environment
> variable from the following test.
This is intentional; I wrote that last sentence ("This ensures...") with
an implied "[When set], this ensures..." at the beginning of it. IOW, if
you wanted to run the test suite with primarily in-memory reverse
indexes, you still could.
> Replacing the line with GIT_TEST_NO_WRITE_REV_INDEX=1 would keep us
> testing the from-scratch case as a side-effect in other tests.
>
> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> > index b098e10f52..e9fbfb6345 100755
> > --- a/ci/run-build-and-tests.sh
> > +++ b/ci/run-build-and-tests.sh
> > @@ -27,7 +27,6 @@ linux-TEST-vars)
> > export GIT_TEST_MULTI_PACK_INDEX=1
> > export GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1
> > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
> > - export GIT_TEST_WRITE_REV_INDEX=1
> > export GIT_TEST_CHECKOUT_WORKERS=2
> > ;;
I wasn't quite sure what to do here. On one hand, sticking
GIT_TEST_NO_WRITE_REV_INDEX=1 here would ensure that the linux-TEST-vars
test is still exercising the old code.
Is that desirable? I dunno. On one hand it increases our coverage, but
on the other hand I've always treated this suite as for experimental
features, not older ones.
But I think all things being equal, I'd rather have more CI coverage
than not, so I'll add it back in. Thanks for a sanity check on this one.
Thanks,
Taylor
