| Message ID | 42865d6c6694b9e6b745c328d717ed244dc25a1a.1713324598.git.dsimic@manjaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | format-patch: fix an option coexistence bug and add new --resend option | expand |
On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic <dsimic@manjaro.org> 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.) > Signed-off-by: Dragan Simic <dsimic@manjaro.org> > --- > 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. 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 > +'
On 2024-04-17 08:48, Eric Sunshine wrote: > On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic <dsimic@manjaro.org> > 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 <dsimic@manjaro.org> >> --- >> 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 >> +'
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index e22c4ac34e6e..bcf7b633e78f 100755 --- 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_must_fail git format-patch -1 --stdout --rfc -k >patch ' +test_expect_success '--resend' ' + cat >expect <<-\EOF && + Subject: [PATCH RESEND 1/1] header with . in it + EOF + git format-patch -n -1 --stdout --resend >patch && + grep "^Subject:" patch >actual && + test_cmp expect actual +' + +test_expect_success '--resend does not overwrite prefix' ' + cat >expect <<-\EOF && + Subject: [PATCH RFC RESEND 1/1] header with . in it + EOF + git -c format.subjectPrefix="PATCH RFC" \ + format-patch -n -1 --stdout --resend >patch && + grep "^Subject:" patch >actual && + test_cmp expect actual +' + +test_expect_success '--resend is argument order independent' ' + cat >expect <<-\EOF && + Subject: [PATCH RFC RESEND 1/1] header with . in it + EOF + git format-patch -n -1 --stdout --resend \ + --subject-prefix="PATCH RFC" >patch && + grep "^Subject:" patch >actual && + test_cmp expect actual +' + +test_expect_success '--resend and -k cannot be used together' ' + test_must_fail git format-patch -1 --stdout --resend -k >patch +' + +test_expect_success '--rfc and --resend cannot be used together' ' + test_must_fail git format-patch -1 --stdout --rfc --resend >patch +' + test_expect_success '--from=ident notices bogus ident' ' test_must_fail git format-patch -1 --stdout --from=foo >patch '
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. Signed-off-by: Dragan Simic <dsimic@manjaro.org> --- t/t4014-format-patch.sh | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)