Re: [PATCH 3/3] commit: add an option the reword HEAD

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 21/09/2020 20:27, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>>>> diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
>>>> @@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '
>>>> +test_reword_opt () {
>>>> +       test_expect_success C_LOCALE_OUTPUT "--reword incompatible with $1" "
>>>> +               echo 'fatal: cannot combine --reword with $1' >expect &&
>>>> +               test_must_fail git commit --reword $1 2>actual &&
>>>> +               test_cmp expect actual
>>>> +       "
>>>> +}
>>> These error messages are subject to localization, so you'd want to
>>> use
>>> test_i18ncmp() here, I think.
>>> Same comment for other new tests.
>>
>> I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather
>> than grep so I could check the exact output.
> 
> I do not think it is a good idea.  Dropping the C_LOCALE_OUTPUT
> prerequisite and using test_i18ncmpw would be more appropriate.
> 
> A test run without GIT_TEST_GETTEXT_POISON will do the byte-for-byte
> comparison like test_cmp.  It is only the poison test, whose purpose
> is to catch commands that by mistake translated their messages, that
> would want to mark a test that checks end-user facing messages like
> this one as special with test_i18ncmp.

Thanks I wasn't aware of test_i18ncmp

>> ... I should probably check that nothing is printed to stdout in
>> these tests
> 
> Perhaps, but that is not the point of "do we diagnose options thare
> are incompatble with --reword?" test.

I think it depends if one views the test as checking "do we diagnose 
options there are incompatible with --reword?" or "what do we show the 
user when there are options that are incompatible with --reword". For 
the former we just want to check that the correct error message is 
printed, for the latter we want to check that only what we expect to be 
printed is actually printed.

Best Wishes

Phillip