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

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <christian.couder@gmail.com> writes:

> Obviousness is often not the same for everybody.

... which you just learned---what you thought obvious turns out to
be not so obvious after all, so you adjust to help your readers.

>> In this particular case it even feels as if this test is not even testing
>> what it should test at all:
>>
>> - it should verify that all of the commits in the first parent lineage are
>>   part of the list
>
> It does that.
>
>> - it should verify that none of the other commits are in the list
>
> It does that too.

But the point is it does a lot more by insisting exact output.  For
example, the version I reviewed had a two "expected output", and
said that the actual output must match either one of them.  I guess
it was because there were two entries with the same distance and we
cannot rely on which order they appear in the result?  If a test
history gained another entry with the same distance, then would we
need 6 possible expected output because we cannot rely on the order
in which these three come out?

That was the only thing I was complaining about.  Dscho gave me too
much credit and read a lot more good things than what I actually
meant to say ;-).

>> And that is really all there is to test.

Another is that "rev-list --bisect-all" promises that the entries
are ordered by the distance value.  So taking the above three
points, perhaps

	cat >expect <<EOF &&
	... as written in one of the expect list in Tiago's patch
	EOF

	# Make sure we have the same entries, nothing more, nothing less
	git rev-list --bisect-all $other_args >actual &&
	sort actual >actual.sorted &&
	sort expect >expect.sorted &&
	test_cmp expect.sorted actual.sorted

	# Make sure the entries are sorted in the dist order
	sed -e 's/.*(dist=\([1-9]*[0-9]\)).*/\1/' actual >actual.dists &&
	sort actual.dists >actual.dists.sorted &&
	test_cmp actual.dists.sorted actual.dists

is what I would have expected.