Re: [RFC/PATCH] tests: add initial bash completion tests

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 6, 2012, 21:42 UTC
Message-ID: <7vsjggzgd9.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAMP44s0xAvc9aTeBP81tXhX6Q67+7cQ-51C1AWrKPN7bc7=JXQ@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 16 quoted lines

> On Fri, Apr 6, 2012 at 11:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:
>>
>>> On Fri, Apr 06, 2012 at 10:28:39PM +0300, Felipe Contreras wrote:
>>>
>>>> Something is better than nothing.
>>>
>>> Yes, but...
>>
>> ;-)
>>
>> This is a good example that sometimes something is worse than nothing,
>> unless watched carefully by a competent reviewer.
>
> And this is a good example of why you shouldn't blindly trust what a
> 'competent reviewer' says, as I'm pretty sure the comment is wrong.

We run the tests under whatever is configured as SHELL_PATH, so whatever you have on #! line does not matter much (except as a documentation).

But it would not make any sense to running the bash completion tests if the shell that is running the test script is *not* bash, would it? That is the point Peff made---the primary thing his message cared about is not to cause "make test" fail when your build does not use bash to run tests.

And that seems to have escaped you.

> But hey, if you prefer nothing, fine, drop this patch;...

Adding a test for bash completion is a good thing, and blindly modifying the completion script without having good tests is a bad idea.

Just add the tests in the right way. Don't break tests for non-bash users.