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

[Junio C Hamano <gitster@pobox.com>]

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.

>> +	cat actual | replace_variable_fields \
>> +	>actual-headers &&

Do not cat a single file into a pipe.  You can instead redirect out
of the file to whatever is reading from the pipe.  I.e.

	replace_variable_fields <actual >actual-headers &&

>> +	test_cmp expected-headers actual-headers
>> +'

OK.  We make sure the presence and the order of the fields in the
output just like all the other tests in this file do (which I think
may be a bit too much---there is no strong reason to insist that
"Subject:" comes before or after "Date:" or is spelled "Subject:"
and not "subject:" or "SUBJECT:"---but that is a problem shared with
many other existing tests in this file and this patch is not making
it much worse).

>>  for enc in 7bit 8bit quoted-printable base64
>>  do
>>  	test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '
>
> As Junio and I discussed in the v5 2/2 patch review, here we may want to
> do something like this: Add a custom header to the SMTP envelope and then make
> sure that that is present when the hook checks $2.

Adding a custom header test is also fine, but I am OK with what we
see above, to verify the headers just the same way as existing
tests.