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

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

 



Am 11.06.2012 17:14, schrieb Junio C Hamano:
> Johannes Sixt <j6t@xxxxxxxx> 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
--
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]