[PATCH 9/9] trailer: add tests for "git interpret-trailers"
Signed-off-by: Christian Couder --- t/t7513-interpret-trailers.sh | 206 ++ 1 file changed, 206 insertions(+) create mode 100755 t/t7513-interpret-trailers.sh diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh new file mode 100755 index 000..beb58fe --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,206 @@ +#!/bin/sh +# +# Copyright (c) 2013 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +cat >basic_message <<'EOF' +subject + +body +EOF + +cat >complex_message_body <<'EOF' +my subject + +my body which is long +and contains some special +chars like : = ? ! + +EOF + +# Do not remove trailing spaces below! +cat >complex_message_trailers <<'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF + +test_expect_success 'without config' ' + printf "ack: Peff\nReviewed-by: \nAcked-by: Johan\n" >expected && + git interpret-trailers "ack = Peff" "Reviewed-by" "Acked-by: Johan" >actual && + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + printf "ack: Peff\nAcked-by: Johan\n" >expected && + git interpret-trailers --trim-empty "ack = Peff" "Reviewed-by" "Acked-by: Johan" "sob:" >actual && + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key "Acked-by: " && + printf "Acked-by: Peff\n" >expected && + git interpret-trailers --trim-empty "ack = Peff" >actual && + test_cmp expected actual && + git interpret-trailers --trim-empty "Acked-by = Peff" >actual && + test_cmp expected actual && + git interpret-trailers --trim-empty "Acked-by :Peff" >actual && + test_cmp expected actual +' + +test_expect_success 'with config setup and = sign' ' + git config trailer.ack.key "Acked-by= " && + printf "Acked-by= Peff\n" >expected && + git interpret-trailers --trim-empty "ack = Peff" >actual && + test_cmp expected actual && + git interpret-trailers --trim-empty "Acked-by= Peff" >actual && + test_cmp expected actual && + git interpret-trailers --trim-empty "Acked-by : Peff" >actual && + test_cmp expected actual +' + +test_expect_success 'with config setup and # sign' ' + git config trailer.bug.key "Bug #" && + printf "Bug #42\n" >expected && + git interpret-trailers --trim-empty "bug = 42" >actual && + test_cmp expected actual +' + +test_expect_success 'with commit basic message' ' + git interpret-trailers --infile basic_message >actual && + test_cmp basic_message actual +' + +test_expect_success 'with commit complex message' ' + cat complex_message_body complex_message_trailers >complex_message && + cat complex_message_body >expected && + printf "Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \n" >>expected && + git interpret-trailers --infile complex_message >actual && + test_cmp expected actual +' + +test_expect_success 'with commit complex message and args' ' + cat complex_message_body >expected && + printf "Fixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \nBug #42\n" >>expected && + git interpret-trailers --infile complex_message "ack: Peff" "bug: 42" >actual && + test_cmp expected actual +' + +test_expect_success 'with commit complex message, args and --trim-empty' ' + cat complex_message_body >expected && + printf "Acked-by= Peff\nBug #42\n" >>expected && + git interpret-trailers --trim-empty --infile complex_message "ack: Peff" "bug: 42" >actual && + test_cmp expected actual +' + +test_expect_success 'using "where = before"' ' + git config trailer.bug.where "before" && + cat complex_message_body >expected && + printf "Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \n" >>expected && + git interpret-trailers --infile complex_message "ack: Peff" "bug: 42" >actual && + test_cmp expected actual +' + +test_expect_success 'using "where = before" for a token in the middle of infile' ' + git config trailer.review.key "Reviewed-by:" && + git config trailer.review.where "before" && + cat complex_message_body >expected && + printf "Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: Johan\nReviewed-by: \nSigned-off-by: \n" >>expected && + git interpret-trailers --infile complex_message "ack: Peff" "bug: 42" "review: Johan" >actual && + test_cmp expected actual +' + +test_expect_success 'using "where = before" and --trim-empty' ' + cat complex_message_body >expected && + printf "Bug #46\nBug #42\nAcked-by= Peff\nReviewed-by: Johan\n" >>expected && + git interpret-trailers --infile complex_message --trim-empty "ack: Peff" "bug: 42" "review: Johan" "Bug: 46" >actual && + test_cmp expected actual +' + +test_expect_success 'the defau
Re: [PATCH 9/9] trailer: add tests for "git interpret-trailers"
Christian Couder writes: > +# Do not remove trailing spaces below! > +cat >complex_message_trailers <<'EOF' > +Fixes: > +Acked-by: > +Reviewed-by: > +Signed-off-by: > +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' <<-\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] trailer: add tests for "git interpret-trailers"
On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: > Christian Couder writes: > > > +# Do not remove trailing spaces below! > > +cat >complex_message_trailers <<'EOF' > > +Fixes: > > +Acked-by: > > +Reviewed-by: > > +Signed-off-by: > > +EOF > > Just a hint. I think it is far safer and robust over time to do > something like this: > > sed -e 's/ Z$/ /' <<-\EOF > Fixes: Z > Acked-by: Z > EOF > > instead of a comment, which can warn human developers but does not > do anything to prevent their editors' auto-fix features from kicking > in. This, but for simplicity, since every line needs the trailing space, why not just use 's/$/ /' and drop the ' Z' on every line? - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] trailer: add tests for "git interpret-trailers"
Josh Triplett writes: > On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: >> Christian Couder writes: >> >> > +# Do not remove trailing spaces below! >> > +cat >complex_message_trailers <<'EOF' >> > +Fixes: >> > +Acked-by: >> > +Reviewed-by: >> > +Signed-off-by: >> > +EOF >> >> Just a hint. I think it is far safer and robust over time to do >> something like this: >> >> sed -e 's/ Z$/ /' <<-\EOF >> Fixes: Z >> Acked-by: Z >> EOF >> >> instead of a comment, which can warn human developers but does not >> do anything to prevent their editors' auto-fix features from kicking >> in. > > This, but for simplicity, since every line needs the trailing space, why > not just use 's/$/ /' and drop the ' Z' on every line? > > > > - Josh Triplett A few reasons: - The "everybody will have a single SP at the end" may or may not last forever; - With your scheme, if you already had _one_ trailing SPs in the input, it would be hard to spot in the source; - It makes it visually very clear that we expect a SP after these colons. This is especially true if you replace 'Z' with something more readable (e.g. '|'). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] trailer: add tests for "git interpret-trailers"
On Mon, Dec 30, 2013 at 12:46:33PM -0800, Junio C Hamano wrote: > Josh Triplett writes: > > > On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: > >> Christian Couder writes: > >> > >> > +# Do not remove trailing spaces below! > >> > +cat >complex_message_trailers <<'EOF' > >> > +Fixes: > >> > +Acked-by: > >> > +Reviewed-by: > >> > +Signed-off-by: > >> > +EOF > >> > >> Just a hint. I think it is far safer and robust over time to do > >> something like this: > >> > >>sed -e 's/ Z$/ /' <<-\EOF > >> Fixes: Z > >> Acked-by: Z > >> EOF > >> > >> instead of a comment, which can warn human developers but does not > >> do anything to prevent their editors' auto-fix features from kicking > >> in. > > > > This, but for simplicity, since every line needs the trailing space, why > > not just use 's/$/ /' and drop the ' Z' on every line? > > > > > > > > - Josh Triplett > > A few reasons: > > - The "everybody will have a single SP at the end" may or may not >last forever; Trivially fixed if that ever changes, but given the nature of all of these, that seems unlikely. > - With your scheme, if you already had _one_ trailing SPs in the >input, it would be hard to spot in the source; Git makes them quite difficult to miss. :) - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] trailer: add tests for "git interpret-trailers"
Josh Triplett writes: >> - The "everybody will have a single SP at the end" may or may not >>last forever; > > Trivially fixed if that ever changes, but given the nature of all of > these, that seems unlikely. Why? Because we encourage to write tests that are expected to find breakages, some of these test vector lines may have to show a broken line that lacks SP after label + colon. >> - With your scheme, if you already had _one_ trailing SPs in the >>input, it would be hard to spot in the source; > > Git makes them quite difficult to miss. :) That is irrelevant, isn't it? This is about protecting the source in the editor, before you run "git show --whitespace=trailing-space", "git diff --check", etc. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] trailer: add tests for "git interpret-trailers"
On Mon, Dec 30, 2013 at 01:05:25PM -0800, Junio C Hamano wrote: > Josh Triplett writes: > >> - With your scheme, if you already had _one_ trailing SPs in the > >>input, it would be hard to spot in the source; > > > > Git makes them quite difficult to miss. :) > > That is irrelevant, isn't it? > > This is about protecting the source in the editor, before you run > "git show --whitespace=trailing-space", "git diff --check", etc. That was exactly my point: such lines shouldn't exist, and rather than including the trailing space and following it with a character that then needs removing, it seems more sensible to me to omit the trailing space and insert it via an almost identical sed line. Git already helps ensure that trailing space won't exist on *any* line, including those; I don't see how an extra character after the space (making it no longer trailing space) makes it any more or less likely that those lines would have trailing space. In any case, I don't care enough to argue the point further; it was just a style suggestion. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
