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. -- Regards, Luben
