Re: [PATCH 2/6] t1414: document some reflog-walk oddities

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 5, 2017, 22:45 UTC
Message-ID: <xmqqa84ih5ao.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170705212757.3ygxiasbi2e7v27p@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 25 quoted lines

>> > +test_expect_failure 'walking multiple reflogs shows both' '
>> > +	{
>> > +		do_walk HEAD &&
>> > +		do_walk side
>> > +	} >expect &&
>> > +	do_walk HEAD side >actual &&
>> > +	test_cmp expect actual
>> > +'
>> 
>> I somehow find this one a bit iffy.  
>> 
>> The order that commits appear in the "walk from HEAD and side at the
>> same time" may want to be different from what this test expects.
>> "rev-list --since=3.days -g master next", for example, would want to
>> refrain from reading the reflog of 'master' for all 90 days before
>> switching to the reflog of 'next', no?
>
> I did make the ordering intentional. We process each reflog sequentially
> in turn. I don't think it would be wrong to interleave them by date, but
> I actually think it's useful for somebody who switches that behavior to
> consciously update the test. Maybe it's worth calling out in the commit
> message.
>
> I stopped short of actually documenting and guaranteeing that behavior
> to the user. I don't know how comfortable we would be changing it later.

I somehow feel that the "showing all entries from HEAD and then showing all from side" is simply a buggy behaviour. I do not think our users would terribly mind if we changed it later. But I may be missing the reason why (sometimes?) the sequential behaviour may be useful.

> (As an aside, I also prowled through the documentation looking for any
> guarantees we make to the user about the fake-parent thing, but I
> couldn't find any. So I considered its user-visible effects an unwanted
> side effect of the implementation).

Yes, I think the corner cases you documented here in these tests are not something we desired to have in the first place. Rather they are merely fallouts from the hackyness of the implementation.