On Wed, Sep 11, 2019 at 10:07:48PM -0400, Taylor Blau wrote:
> > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> > index 8210f63d41..7601664b74 100755
> > --- a/t/t5500-fetch-pack.sh
> > +++ b/t/t5500-fetch-pack.sh
> > +# - we must use protocol v2, because it handles the "have" negotiation before
> > +# processing the shallow direectives
s/ee/e/
> > +#
> > +test_expect_success 'shallow since with commit graph and already-seen commit' '
> > + test_create_repo shallow-since-graph &&
> > + (
>
> I'm not sure if this same-level indentation is common, or you're missing
> an extra tab here. Either way.
>
> > + cd shallow-since-graph &&
> > + test_commit base &&
> > + test_commit master &&
> > + git checkout -b other HEAD^ &&
> > + test_commit other &&
> > + git commit-graph write --reachable &&
> > + git config core.commitGraph true &&
>
> Another nit, but do you have any thoughts about using 'test_config' here
> instead of a pure 'git config'? I don't think that it really would
> matter much (since none of the other tests hopefully have anything to do
> with commit-graph, and doubly so if it is enabled by default, _and_
> since you're using your own repository), but anyway.
We can't simply replace that 'git config' command with 'test_config',
because it is implemented using 'test_when_finished', which doesn't
work in a subshell [1]. What we could do is:
test_create_repo shallow-since-graph &&
test_config -C shallow-since-graph core.commitGraph true &&
(
cd shallow-since-graph &&
....
Or we could entirely avoid the subshell by passing '-C
shallow-since-graph' to every single command... [2]
However, since this repo was specifically created for this test, it
doesn't really matter in what state it's left behind, so I don't think
it's worth it.
[1] 0968f12a99 (test-lib-functions: detect test_when_finished in
subshell, 2015-09-05)
[2] Or bite the bullet, and declare that every test case shall start
in $TRASH_DIRECTORY.
