Re: [PATCHv4] rebase [-i --exec | -ix] <CMD>...

From: Johannes Sixt <j6t@kdbg.org>
Date: Jun 12, 2012, 18:55 UTC
Message-ID: <4FD790AF.5070108@kdbg.org>
In-Reply-To: <7vpq9598yb.fsf@alter.siamese.dyndns.org>

Am 11.06.2012 17:14, schrieb Junio C Hamano:

Show 17 quoted lines

> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Am 10.06.2012 12:44, schrieb Lucien Kong:
>>> +test_expect_success 'rebase -i --exec without <CMD> shows error message and usage' '
>>> +	git reset --hard execute &&
>>> +	test_must_fail git rebase -i --exec 2>actual &&
>>> +	sed '1d' actual >tmp &&
>>> +	mv tmp actual &&
>>> +	test_must_fail git rebase -h >expected &&
>>> +	test_cmp expected actual &&
>>> +	git checkout master
>>> +'
>>
>> IMO, it is more important to check that the error message is present
>> rather than whether the usage blurb is correct. But since the error is
>> generated by the option parsing machinery, it is probably sufficient to
>> check *only* for failure, and don't verify the output at all.

I changed my mind. If option parsing regresses in such a way that --exec suddenly works without command, then the 'test_must_fail git rebase ...' could pass because there is a failure much later in the rebase process, which we would not detect if we do not check the output. It is therefore good to check whether the expected failure happens during option parsing.

Checking the usage blurb rather than the error message even has advantages: it is immune to changes of the error messages and to i18n poisoning.

In this light, the version you have queued is fine.

-- Hannes