From: Dragan Simic Date: Wed, 17 Apr 2024 07:20:57 GMT Subject: Re: [PATCH 4/4] t4014: add tests to cover --resend option and its exclusivity Message-ID: <82846020aedfdc5eadf5bc8349575ec1@manjaro.org> In-Reply-To: On 2024-04-17 08:48, Eric Sunshine wrote: > On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic > wrote: >> Add a few new tests to the t4014 that cover the --resend command-line >> option >> for "git format-patch", which include the tests for its exclusivity >> with the >> already existing -k and --rfc command-line options. > > I'd recommend squashing this patch into [3/4] which introduces the > --resend option since it's easier to review the tests when the code > which is being tested is still fresh in one's mind. (For the same > reason, reviewers like to see documentation added in the same patch > which changes the code since it's easier to verify that the > documentation matches the implementation while it's fresh in the > mind.) I'm fine with that. Squashing these two patches together might also be good for bisecting later, if need arises. >> Signed-off-by: Dragan Simic >> --- >> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh >> @@ -1401,6 +1401,43 @@ test_expect_success '--rfc and -k cannot be >> used together' ' >> +test_expect_success '--resend' ' >> + cat >expect <<-\EOF && >> + Subject: [PATCH RESEND 1/1] header with . in it >> + EOF > > In all of the new tests, since it's just a single line body, it could > just as easily be created with `echo`: > > echo "Subject: [PATCH RESEND 1/1] header with . in it" >expect && > > On the other hand, if you're following precedent in this script, then > using a here-doc may be just fine. I agree that using "echo ..." would be nicer. Though, I just wanted to follow the already existing tests for consistency, which may actually outweigh a nicer approach. > At any rate it's somewhat subjective and not worth a reroll. > >> + git format-patch -n -1 --stdout --resend >patch && >> + grep "^Subject:" patch >actual && >> + test_cmp expect actual >> +'