On Tue, Sep 12, 2023 at 7:07 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
> > +# We create 5 commits and move them to the alt directory and
> > +# create 5 more commits which will stay in the main odb.
> > +test_expect_success 'create repository and alternate directory' '
> > + git init main &&
>
> We don't necessarily have to initialize a repository, as the test suite
> already does so for us. So we may want to write this instead as:
>
> test_commit_bulk 5 &&
> git clone --reference=. --shared . alt &&
> test_commit_bulk -C alt --start=6 5
>
I was trying to use the env variable `GIT_ALTERNATE_OBJECT_DIRECTORIES` and
get hence ended up creating a new repository. But I really like the
convenience functions
that you've suggested below. With that, this seems like the way to go.
> > +# when the alternate odb is provided, all commits are listed along with the boundary
> > +# commit.
> > +test_expect_success 'rev-list passes with alternate object directory' '
> > + GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/alt git -C main rev-list HEAD >actual &&
> > + test_stdout_line_count = 10 cat actual &&
> > + grep $BOUNDARY_COMMIT actual
> > +'
>
> Here, I think we'd want to make sure that we have not just 10 lines of
> output, but that they are the 10 that we expect, like so:
>
> git -C alt rev-list --all --objects --no-object-names >actual.raw &&
> {
> git rev-list --all --objects --no-object-names &&
> git -C alt rev-list --all --objects --no-object-names --not \
> --alternate-refs
> } >expect.raw &&
> sort actual.raw >actual &&
> sort expect.raw >expect &&
> test_cmp expect actual
>
> When reviewing this and tweaking some of the tests locally, I found it
> useful to have some convenience functions like "hide_alternates" and
> "show_alternates" to control whether or not "alt" could see its
> alternate or not.
>
> From my review locally, the resulting changes (which can be applied
> directly on top of your patch here look like):
>
This is much better. I didn't know about `test_oid_to_path` and
`test_when_finished`, and overall
your patch looks much nicer and is more thorough in the testing. I'll
add it to the next version.
Will wait a day or two for more feedback before I submit v3.
Thanks again for your review and the patch :)
