Subject: Re: [PATCH 08/11] t4207: delete replace references via git-update-ref(1)

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 23 Oct 2023 15:58:35 +0200

On Wed, Oct 18, 2023 at 03:27:11PM +0200, Han-Wen Nienhuys wrote:
> On Wed, Oct 18, 2023 at 7:35 AM Patrick Steinhardt <[email-masked]> wrote:
> >
> > In t4207 we set up a set of replace objects via git-replace(1). Because
> > these references should not be impacting subsequent tests we also set up
> > some cleanup logic that deletes the replacement references via a call to
> > `rm -rf`. This reaches into the internal implementation details of the
> > reference backend and will thus break when we grow an alternative refdb
> > implementation.
> >
> > Refactor the tests to delete the replacement refs via Git commands so
> > that we become independent of the actual refdb that's in use. As we
> > don't have a nice way to delete all replacements or all references in a
> > certain namespace, we opt for a combination of git-for-each-ref(1) and
> > git-update-ref(1)'s `--stdin` mode.
> 
> There is a test helper that can directly access the ref database,
> t/helper/test-ref-store.c.
> 
> If you use that manipulate refs for testing purposes, you make the
> test independent of behavior git-for-each-ref/git-update-ref, which is
> what you want for testing replace-objects?

Is there any specific reason why we shouldn't be using
git-for-each-ref(1) or git-update-ref(1) here? Neither of those commands
are part of the system under test, as we rather care about git-log(1)
here. So as those commands are already being verified in other tests I
think it should be fine to assume that they work as intended here.

Happy to hear differing viewpoints though.

Patrick