* [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case @ 2018-09-11 20:21 Derrick Stolee via GitGitGadget 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-09-11 20:21 UTC (permalink / raw) To: git; +Cc: Junio C Hamano In es/format-patch-rangediff, we added a '--range-diff' option to git-format-patch to automatically add a range-diff. We also added an option to write the diff as commentary to a single patch submission. However, this check was not tested. I discovered this test gap by running 'make coverage-test coverage-report' on 'next' and then comparing the uncovered lines with those in the diff between 'master' and 'next'. I have a script that automates this process, and I'm still working on polishing it. You can see an earlier version at [1]. Based on es/format-patch-rangediff Cc: sunshine@sunshine.co Cc: peff@peff.net [1] https://github.com/derrickstolee/git/blob/coverage/contrib/coverage-diff.shA bash script to report uncovered lines that were added in a diff. Derrick Stolee (1): t3206-range-diff.sh: cover single-patch case t/t3206-range-diff.sh | 5 +++++ 1 file changed, 5 insertions(+) base-commit: 40ce41604daf200cdc85abded0133d40faafc2f8 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-37%2Fderrickstolee%2Frange-diff-test-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-37/derrickstolee/range-diff-test-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/37 -- gitgitgadget ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-11 20:21 [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case Derrick Stolee via GitGitGadget @ 2018-09-11 20:21 ` Derrick Stolee via GitGitGadget 2018-09-11 20:58 ` Junio C Hamano ` (2 more replies) 2018-09-11 20:29 ` [PATCH 0/1] Add coverage for 'git format-patch --range-diff' " Derrick Stolee ` (2 subsequent siblings) 3 siblings, 3 replies; 11+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-09-11 20:21 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Derrick Stolee From: Derrick Stolee <dstolee@microsoft.com> The commit 40ce4160 "format-patch: allow --range-diff to apply to a lone-patch" added the ability to see a range-diff as commentary after the commit message of a single patch series (i.e. [PATCH] instead of [PATCH X/N]). However, this functionality was not covered by a test case. Add a simple test case that checks that a range-diff is written as commentary to the patch. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- t/t3206-range-diff.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 3d7a2d8a4d..05ef3263d2 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -154,4 +154,9 @@ do ' done +test_expect_success 'format-patch --range-diff as commentary' ' + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && + grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget @ 2018-09-11 20:58 ` Junio C Hamano 2018-09-11 21:11 ` Junio C Hamano 2018-09-11 21:09 ` Stefan Beller 2018-09-11 21:34 ` Eric Sunshine 2 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2018-09-11 20:58 UTC (permalink / raw) To: Derrick Stolee via GitGitGadget; +Cc: git, Derrick Stolee "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Derrick Stolee <dstolee@microsoft.com> > > The commit 40ce4160 "format-patch: allow --range-diff to apply to > a lone-patch" added the ability to see a range-diff as commentary > after the commit message of a single patch series (i.e. [PATCH] > instead of [PATCH X/N]). However, this functionality was not > covered by a test case. > > Add a simple test case that checks that a range-diff is written as > commentary to the patch. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/t3206-range-diff.sh | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > index 3d7a2d8a4d..05ef3263d2 100755 > --- a/t/t3206-range-diff.sh > +++ b/t/t3206-range-diff.sh > @@ -154,4 +154,9 @@ do > ' > done > > +test_expect_success 'format-patch --range-diff as commentary' ' > + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && > + grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" Isn't "grep -A" GNUism? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-11 20:58 ` Junio C Hamano @ 2018-09-11 21:11 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2018-09-11 21:11 UTC (permalink / raw) To: Derrick Stolee via GitGitGadget; +Cc: git, Derrick Stolee Junio C Hamano <gitster@pobox.com> writes: >> +test_expect_success 'format-patch --range-diff as commentary' ' >> + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && >> + grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" > > Isn't "grep -A" GNUism? Sorry for short-write(2) X-<. Perhaps sed -ne "/^---$/,+1/s/^Range-diff:/&/p" or something along that line. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget 2018-09-11 20:58 ` Junio C Hamano @ 2018-09-11 21:09 ` Stefan Beller 2018-09-11 21:34 ` Eric Sunshine 2 siblings, 0 replies; 11+ messages in thread From: Stefan Beller @ 2018-09-11 21:09 UTC (permalink / raw) To: gitgitgadget; +Cc: git, Junio C Hamano, Derrick Stolee On Tue, Sep 11, 2018 at 1:21 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > The commit 40ce4160 "format-patch: allow --range-diff to apply to > a lone-patch" added the ability to see a range-diff as commentary > after the commit message of a single patch series (i.e. [PATCH] > instead of [PATCH X/N]). However, this functionality was not > covered by a test case. > > Add a simple test case that checks that a range-diff is written as > commentary to the patch. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/t3206-range-diff.sh | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > index 3d7a2d8a4d..05ef3263d2 100755 > --- a/t/t3206-range-diff.sh > +++ b/t/t3206-range-diff.sh > @@ -154,4 +154,9 @@ do > ' > done > > +test_expect_success 'format-patch --range-diff as commentary' ' > + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && This is an interesting use of range-diff, as it basically tells us "Range-diff: This is a new patch", but it works to make sure there is a range diff section. (I shortly wondered if we would ever omit the range diff for "obvious" cases or word it differently) > + grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" So the first grep finds the three dashes, presumably those after the commit message/ but others as well, e.g. in --- a/<path> +++ b/<path> and then the second grep should find the string "Range-diff". By having the greps chained with a pipe, only one return code can be delivered to the test suite, and as we get the last commands return code, we get reported if we found the string in the preselected part. I was wondering if we could get away with just one command to check for that multi line pattern sed -n -e '/---/,/^Range/p' actual seems to detect that pattern, and prints from there on to the rest of the file. > +' > + > test_done > -- > gitgitgadget ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget 2018-09-11 20:58 ` Junio C Hamano 2018-09-11 21:09 ` Stefan Beller @ 2018-09-11 21:34 ` Eric Sunshine 2018-09-12 14:20 ` Derrick Stolee 2 siblings, 1 reply; 11+ messages in thread From: Eric Sunshine @ 2018-09-11 21:34 UTC (permalink / raw) To: gitgitgadget; +Cc: Git List, Junio C Hamano, Derrick Stolee On Tue, Sep 11, 2018 at 4:26 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > The commit 40ce4160 "format-patch: allow --range-diff to apply to > a lone-patch" added the ability to see a range-diff as commentary > after the commit message of a single patch series (i.e. [PATCH] > instead of [PATCH X/N]). However, this functionality was not > covered by a test case. > > Add a simple test case that checks that a range-diff is written as > commentary to the patch. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > @@ -154,4 +154,9 @@ do > +test_expect_success 'format-patch --range-diff as commentary' ' > + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && > + grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" > +' Aside from Junio's and Stefan's comments... Patch 6/14 [1], in addition to checking that a solo patch contains an interdiff, takes the extra step of checking that individual patches _don't_ contain an interdiff when --cover-letter is used. I wonder if the same should be done here, though I don't feel too strongly about it. If you do go that route, it might make sense to move this test to t4014 as neighbor to the --interdiff tests. The reason 10/14 [2] added the "git format-patch --range-diff" test to t3206 instead of t4014 was so it could do a thorough check of the embedded range-diff by re-using the specially crafted test repo set up by t3206. Your new test is much looser, thus could be moved alongside the --interdiff tests. Not a big deal, though. Either way is fine. Thanks for working on this. [1]: https://public-inbox.org/git/20180722095717.17912-7-sunshine@sunshineco.com/ [2]: https://public-inbox.org/git/20180722095717.17912-11-sunshine@sunshineco.com/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-11 21:34 ` Eric Sunshine @ 2018-09-12 14:20 ` Derrick Stolee 0 siblings, 0 replies; 11+ messages in thread From: Derrick Stolee @ 2018-09-12 14:20 UTC (permalink / raw) To: Eric Sunshine, gitgitgadget; +Cc: Git List, Junio C Hamano, Derrick Stolee On 9/11/2018 5:34 PM, Eric Sunshine wrote: > On Tue, Sep 11, 2018 at 4:26 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> The commit 40ce4160 "format-patch: allow --range-diff to apply to >> a lone-patch" added the ability to see a range-diff as commentary >> after the commit message of a single patch series (i.e. [PATCH] >> instead of [PATCH X/N]). However, this functionality was not >> covered by a test case. >> >> Add a simple test case that checks that a range-diff is written as >> commentary to the patch. >> >> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> >> --- >> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh >> @@ -154,4 +154,9 @@ do >> +test_expect_success 'format-patch --range-diff as commentary' ' >> + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && >> + grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" >> +' > Aside from Junio's and Stefan's comments... > > Patch 6/14 [1], in addition to checking that a solo patch contains an > interdiff, takes the extra step of checking that individual patches > _don't_ contain an interdiff when --cover-letter is used. I wonder if > the same should be done here, though I don't feel too strongly about > it. If you do go that route, it might make sense to move this test to > t4014 as neighbor to the --interdiff tests. The reason 10/14 [2] added > the "git format-patch --range-diff" test to t3206 instead of t4014 was > so it could do a thorough check of the embedded range-diff by re-using > the specially crafted test repo set up by t3206. Your new test is much > looser, thus could be moved alongside the --interdiff tests. Not a big > deal, though. Either way is fine. Thanks for working on this. > > [1]: https://public-inbox.org/git/20180722095717.17912-7-sunshine@sunshineco.com/ > [2]: https://public-inbox.org/git/20180722095717.17912-11-sunshine@sunshineco.com/ Thanks for these links! In particular, [2] uses this line to test the inter-diff appears: + test_i18ngrep "^Interdiff:$" 0001-fleep.patch && That's a better way to test, especially with the translation. It would be enough for my needs. Thanks, -Stolee P.S. Resending because apparently I had HTML in the last response ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case 2018-09-11 20:21 [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case Derrick Stolee via GitGitGadget 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget @ 2018-09-11 20:29 ` Derrick Stolee 2018-09-11 21:21 ` Eric Sunshine 2018-09-12 14:31 ` [PATCH v2 " Derrick Stolee via GitGitGadget 3 siblings, 0 replies; 11+ messages in thread From: Derrick Stolee @ 2018-09-11 20:29 UTC (permalink / raw) To: Derrick Stolee via GitGitGadget, git Cc: Junio C Hamano, Jeff King, Eric Sunshine On 9/11/2018 4:21 PM, Derrick Stolee via GitGitGadget wrote: > In es/format-patch-rangediff, we added a '--range-diff' option to > git-format-patch to automatically add a range-diff. We also added an option > to write the diff as commentary to a single patch submission. However, this > check was not tested. > > I discovered this test gap by running 'make coverage-test coverage-report' > on 'next' and then comparing the uncovered lines with those in the diff > between 'master' and 'next'. I have a script that automates this process, > and I'm still working on polishing it. You can see an earlier version at > [1]. > > Based on es/format-patch-rangediff > > Cc: sunshine@sunshine.co > > Cc: peff@peff.net Sorry that these Cc's didn't work (again). Adding them manually and investigating the GGG code. -Stolee ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case 2018-09-11 20:21 [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case Derrick Stolee via GitGitGadget 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget 2018-09-11 20:29 ` [PATCH 0/1] Add coverage for 'git format-patch --range-diff' " Derrick Stolee @ 2018-09-11 21:21 ` Eric Sunshine 2018-09-12 14:31 ` [PATCH v2 " Derrick Stolee via GitGitGadget 3 siblings, 0 replies; 11+ messages in thread From: Eric Sunshine @ 2018-09-11 21:21 UTC (permalink / raw) To: gitgitgadget; +Cc: Git List, Junio C Hamano On Tue, Sep 11, 2018 at 4:26 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > In es/format-patch-rangediff, we added a '--range-diff' option to > git-format-patch to automatically add a range-diff. We also added an option > to write the diff as commentary to a single patch submission. However, this > check was not tested. It was an intentional (though unjustified) oversight. Thanks for working on this. > I discovered this test gap by running 'make coverage-test coverage-report' > on 'next' and then comparing the uncovered lines with those in the diff > between 'master' and 'next'. I have a script that automates this process, > and I'm still working on polishing it. Interesting. Does your script detect that the --creation-factor option added by patch 13/14 [1] also lacks a test? (I'm not necessarily suggesting adding a test at the moment. Even Dscho's git-range-diff series doesn't have a test for "creation factor", so it wouldn't necessarily make sense to add a test only for the git-format-patch case. Instead, we'd want, at some point, to add tests covering both git-range-diff and git-format-patch.) [1]: https://public-inbox.org/git/20180722095717.17912-14-sunshine@sunshineco.com/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/1] Add coverage for 'git format-patch --range-diff' single-patch case 2018-09-11 20:21 [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case Derrick Stolee via GitGitGadget ` (2 preceding siblings ...) 2018-09-11 21:21 ` Eric Sunshine @ 2018-09-12 14:31 ` Derrick Stolee via GitGitGadget 2018-09-12 14:31 ` [PATCH v2 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget 3 siblings, 1 reply; 11+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-09-12 14:31 UTC (permalink / raw) To: git; +Cc: Junio C Hamano In es/format-patch-rangediff, we added a '--range-diff' option to git-format-patch to automatically add a range-diff. We also added an option to write the diff as commentary to a single patch submission. However, this check was not tested. I discovered this test gap by running 'make coverage-test coverage-report' on 'next' and then comparing the uncovered lines with those in the diff between 'master' and 'next'. I have a script that automates this process, and I'm still working on polishing it. You can see an earlier version at [1]. Based on es/format-patch-rangediff Cc: sunshine@sunshine.co Cc: peff@peff.net [1] https://github.com/derrickstolee/git/blob/coverage/contrib/coverage-diff.shA bash script to report uncovered lines that were added in a diff. Derrick Stolee (1): t3206-range-diff.sh: cover single-patch case t/t3206-range-diff.sh | 5 +++++ 1 file changed, 5 insertions(+) base-commit: 40ce41604daf200cdc85abded0133d40faafc2f8 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-37%2Fderrickstolee%2Frange-diff-test-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-37/derrickstolee/range-diff-test-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/37 Range-diff vs v1: 1: 58347a9624 ! 1: 277a4d2bd8 t3206-range-diff.sh: cover single-patch case @@ -22,7 +22,7 @@ +test_expect_success 'format-patch --range-diff as commentary' ' + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && -+ grep -A 1 -e "\-\-\-" actual | grep "Range-diff:" ++ test_i18ngrep "^Range-diff:$" actual +' + test_done -- gitgitgadget ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/1] t3206-range-diff.sh: cover single-patch case 2018-09-12 14:31 ` [PATCH v2 " Derrick Stolee via GitGitGadget @ 2018-09-12 14:31 ` Derrick Stolee via GitGitGadget 0 siblings, 0 replies; 11+ messages in thread From: Derrick Stolee via GitGitGadget @ 2018-09-12 14:31 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Derrick Stolee From: Derrick Stolee <dstolee@microsoft.com> The commit 40ce4160 "format-patch: allow --range-diff to apply to a lone-patch" added the ability to see a range-diff as commentary after the commit message of a single patch series (i.e. [PATCH] instead of [PATCH X/N]). However, this functionality was not covered by a test case. Add a simple test case that checks that a range-diff is written as commentary to the patch. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- t/t3206-range-diff.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 3d7a2d8a4d..6babc0e276 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -154,4 +154,9 @@ do ' done +test_expect_success 'format-patch --range-diff as commentary' ' + git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && + test_i18ngrep "^Range-diff:$" actual +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-12 14:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-11 20:21 [PATCH 0/1] Add coverage for 'git format-patch --range-diff' single-patch case Derrick Stolee via GitGitGadget 2018-09-11 20:21 ` [PATCH 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget 2018-09-11 20:58 ` Junio C Hamano 2018-09-11 21:11 ` Junio C Hamano 2018-09-11 21:09 ` Stefan Beller 2018-09-11 21:34 ` Eric Sunshine 2018-09-12 14:20 ` Derrick Stolee 2018-09-11 20:29 ` [PATCH 0/1] Add coverage for 'git format-patch --range-diff' " Derrick Stolee 2018-09-11 21:21 ` Eric Sunshine 2018-09-12 14:31 ` [PATCH v2 " Derrick Stolee via GitGitGadget 2018-09-12 14:31 ` [PATCH v2 1/1] t3206-range-diff.sh: cover " Derrick Stolee via GitGitGadget
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.