From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 29 Sep 2026 05:45:41 GMT
Subject: Re: [PATCH v3] reflog: fix default expiry periods
Message-ID: <artQhZKf6JuRhmRl@pks.im>
In-Reply-To: <xmqqy0clo2em.fsf@gitster.g>

On Mon, Sep 28, 2026 at 07:50:57AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Thu, Sep 24, 2026 at 05:58:44PM +0000, Pushkar Singh wrote:
> >> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> >> index 8f78cf4b01..93b5b49e1d 100755
> >> --- a/t/t1410-reflog.sh
> >> +++ b/t/t1410-reflog.sh
> >> @@ -153,6 +153,72 @@ test_expect_success 'reflog expire should not barf on an annotated tag' '
> >>  	test_grep ! "error: [Oo]bject .* not a commit" err
> >>  '
> >>  
> >> +test_expect_success 'reflog expire keeps reachable entries for 90 days' '
> >> +	test_when_finished "rm -rf reachable-keep" &&
> >> +	git init reachable-keep &&
> >> +	(
> >> +		cd reachable-keep &&
> >> +		timestamp=$(test-tool date timestamp "60.days.ago") &&
> >
> > Nit: I would've preferred to make this 89 days...
> 
> Dates calculated as 89 days ago from the beginning of today, from
> the end of today, and from this very minute can differ by almost 24
> hours.  Because we are not interested in testing what semantics
> approxidate() implements in test-tool date timestamp, but are
> testing what expiry period reflog expire implements between 30 and
> 90 days, using numbers that are not too close to the edge spares us
> from having to worry about boundary cases we do not care about.
> 
> So I wouldn't have preferred using 89 days there.

Fair enough. I just find it a bit fishy to assert that we "[keep]
reachable entries for 90 days" by checking that we keep it for 60 days
but throw it away after 100 days. THat allows for a very wide range of
values that aren't 90 days.

So even if it shouldn't be 89 days, it could very well have been 88 days
without any risk for test flakiness.

Patrick