On 2023-01-18 11:35, Luben Tuikov wrote: > On 2023-01-18 11:27, Junio C Hamano wrote: >> Luben Tuikov <luben.tuikov@xxxxxxx> writes: >> >>> On 2023-01-17 02:31, Junio C Hamano wrote: >>>> Luben Tuikov <luben.tuikov@xxxxxxx> 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. I've tested using the envelope-sender=<who> and the hook only gets the headers. I've applied the feedback above in patch set v8 including a test for the 2nd argument. The new test will fail if either the supplied argument is not a file or the custom header is not found.
