From: Dragan Simic <dsimic@manjaro.org>
Date: Wed, 17 Apr 2024 12:00:29 GMT
Subject: Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014
Message-ID: <d2a4e3b3a5abbf1fe1e8a164b410edd8@manjaro.org>
In-Reply-To: <e4aa5235-c6ad-45c7-930e-de991cc375f2@app.fastmail.com>

On 2024-04-17 08:33, Kristoffer Haugsbakk wrote:
> On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index e37a1411ee24..e22c4ac34e6e 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order
>> independent' '
>>  	test_cmp expect actual
>>  '
>> 
>> +test_expect_success '--rfc and -k cannot be used together' '
>> +	test_must_fail git format-patch -1 --stdout --rfc -k >patch
> 
> I don’t understand why you redirect to `patch` if you only check the
> exit code. (I don’t expect any stdout since it will fail.)
> 
> Although it would be nice with a text comparison or grep on the stderr
> output to make sure that the command died for the expected reason. But 
> I
> haven’t read the associated code.

Actually, if you agree, we should check both the stdout
and stderr -- the former for emptiness, and the latter for
the expected error message.

>> +'
>> +
>>  test_expect_success '--from=ident notices bogus ident' '
>>  	test_must_fail git format-patch -1 --stdout --from=foo >patch
>>  '