Re: [RFC PATCH v5] Implement --first-parent for git rev-list --bisect

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 3 Jul 2018, Junio C Hamano wrote:

> Tiago Botelho <tiagonbotelho@gmail.com> writes:
> 
> > git rev-list --first-parent --bisect-all F..E >revs &&
> > test_line_count = 9 revs &&
> > for rev in E e1 e2 e3 e4 e5 e6 e7 e8
> > do
> >   grep "^$(git rev-parse $rev) " revs ||
> >   {
> >     echo "$rev not shown" >&2 &&
> >     return 1
> >   }
> > done &&
> > sed -e "s/.*(dist=\([0-9]*\)).*/\1/" revs >actual.dists &&
> > sort -r actual.dists >actual.dists.sorted &&
> > test_cmp actual.dists.sorted actual.dists
> 
> The distance in the current graph might be all lower single-digits,
> but you need "sort -n -r" to be future-proof, as dist=10 would sort
> next to dist=1, perhaps in between dist=1 and dist=2.
> 
> Another thing you missed in my message is that the above does not
> say what distance value each commit should be assigned in the
> history.  Even though the grep loop makes sure that each of E, e1,
> ... e8 appears at least once, and line-count before that ensures
> that the output has 9 entries (and taken together, it guarantees
> that each of these appears not just "at least once", but also
> exactly once), nothing guarantees if they are getting relative
> distance correctly, as the sed strips a bit too much (and that
> relates to my earlier point why starting from a concrete expected
> output and explicitly discard the aspect of the output we do not
> care about before comparison---that way, we can easily tell when the
> code is _designed to_ discard too much).

From my point of view, this indicates that you want to set those exact
dist values in stone. I am not sure I follow that reasoning, as this
particular test case's purpose is to ensure that the `--first-parent`
option works with `--bisect`, not that the distance values match a fixed
expectation.

In short: I disagree that the *exact* values (beyond testing the order)
should be tested *here*.

Ciao,
Dscho