Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:

> On Thu, Jul 8, 2010 at 19:40, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:
>>
>>> +if git grep ile a
>>> +then
>>> +     test_expect_success 'git grep ile a' 'git grep ile a'
>>> +else
>>> +     test_expect_failure 'git grep ile a' 'git grep ile a'
>>> +fi
>>
>> So if command "X" is known to succeed, we run it inside expect_success
>> and if not we run it inside expect_failure?
>>
>> What kind of idiocy is that, I have to wonder...
>
> Well, the point is to normalize the test suite so that we never have
> passing TODO tests if everything's OK.

I do not consider a test that passes under some condition but doesn't
under some other condition "everything is OK".  Marking the test as
"expect failure" as René originally did makes a lot of sense to me.

The quoted patch is even worse as it will _actively_ prevent you from
catching a new error you just introduced while futzing "git grep" on a
platform that used to work.  Your "if" statement will say "ah, grep is
broken", and you will use expect-failure, not because your platform does
not support REG_STARTEND, but because you broke "git grep".

The point of having tests is to help you catch your bugs while you
develop.  A test that turns itself off when the feature it is testing is
broken helps nobody.

So forget about "passing TODO tests", whatever a "TODO test" is.  The
change in question is actively _wrong_.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]