Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook

From: Luben Tuikov <luben.tuikov@amd.com>
Date: Jan 18, 2023, 16:35 UTC
Message-ID: <fa9b1371-0a61-147f-637e-cb09f775fe22@amd.com>
In-Reply-To: <xmqqv8l34xkp.fsf@gitster.g>

On 2023-01-18 11:27, Junio C Hamano wrote:

Show 28 quoted lines

> Luben Tuikov <luben.tuikov@amd.com> writes:
> 
>> On 2023-01-17 02:31, Junio C Hamano wrote:
>>> Luben Tuikov <luben.tuikov@amd.com> writes:
>>>
>>>>> +test_expect_success $PREREQ "--validate hook supports header argument" '
>>>>> +	write_script my-hooks/sendemail-validate <<-\EOF &&
>>>>> +	if test -s "$2"
>>>>> +	then
>>>>> +		cat "$2" >actual
>>>>> +		exit 1
>>>>> +	fi
>>>>> +	EOF
>>>
>>> If "$2" is not given, or an empty "$2" is given, is that an error?
>>> I am wondering if the lack of "else" clause (and the hook exits with
>>> success when "$2" is an empty file) here is intentional.
>>
>> I think we'll always have a $2, since it is the SMTP envelope and headers.
> 
> We write our tests to verify _that_ assumption you have.  A future
> developer mistakenly drops the code to append the file to the
> command line that invokes the hook, and we want our test to catch
> such a mistake.
> 
> Do we really feed envelope?  E.g. if the --envelope-sender=<who> is
> used, does $2 have the "From:" from the header and "MAIL TO" from
> the envelope separately?

I'm not sure--I thought we did, but yes, we should _test_ that we indeed
1) have/get $2, as a non-empty string,
2) it is a non-empty, readable file,
3) contains the test header we included in git-format-patch in the test.

This is what I meant when I wrote "we'll always have $2 ...", not having it is failure of some kind and yes we should test for it.

-- 
Regards,
Luben