Re: [PATCH 1/3] refs/reftable: introduce "reftable.lockTimeout"

[Patrick Steinhardt <ps@pks.im>]

On Tue, Sep 17, 2024 at 01:50:27PM -0400, Eric Sunshine wrote:
> On Tue, Sep 17, 2024 at 1:46 PM karthik nayak <karthik.188@gmail.com> wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> > > +test_expect_success 'ref transaction: retry acquiring tables.list lock' '
> > > +     test_when_finished "rm -rf repo" &&
> > > +     git init repo &&
> > > +     (
> > > +             cd repo &&
> > > +             test_commit initial &&
> > > +             LOCK=.git/reftable/tables.list.lock &&
> > > +             >$LOCK &&
> > > +             {
> > > +                     ( sleep 1 && rm -f $LOCK ) &
> > > +             } &&
> > > +             git -c reftable.lockTimeout=5000 update-ref refs/heads/branch HEAD
> > > +     )
> > > +'
> >
> > Nit: This does stall the test for 1s. Which is slightly annoying when
> > running single tests locally. Couldn't we achieve this by doing `sleep
> > 0.1`?
> 
> `sleep 0.1` is neither POSIX nor portable.

The above test also verifies that the timeout can in fact be overridden.
If we only had `sleep 0.1` we wouldn't be able to reliably verify that,
as the default is a timeout of 100ms.

We also have essentially the same tests in t0601 for packed-refs, I
mostly just copied the logic over from there. It's not exactly pretty,
and I'm not a huge fan of introducing timing sensitive tests, but in the
end I guess it's okayish.

Patrick