Christian Couder <[email protected]> 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.
