Re: [GUILT 07/28] Added test cases for guilt fold.
On Wed, May 7, 2014 at 11:06 PM, Jeff Sipek jef...@josefsipek.net wrote: On Wed, May 07, 2014 at 10:59:56PM +0200, Per Cederqvist wrote: On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek jef...@josefsipek.net wrote: On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote: Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) I feel like we should have *some* content there - most of the time, I care more about the diffs getting folded than the commit message :) I added these tests for a reason: the reproduce a bug in guilt that I found. I'm afraid that having some content might hide the bug I found. (I'm also equally afraid that it might uncover other bugs in guilt, which would delay integration of this patch series. So adding more test cases with content is a good thing to do, but maybe not in this patch series.) Fair enough. I use guilt-fold all the time and it hasn't lost any of my diffs, so I'm happy to defer this until some point in the future. ... for using_diffstat in true false ; do for patcha in empty nonempty ; do for patchb in empty nonempty ; do echo %% $patcha + $patchb (diffstat=$using_diffstat) ${patcha}_patch $patcha ${patchb}_patch $patchb cmd guilt pop $patchb cmd guilt fold $patchb fixup_time_info $patcha cmd list_files cleanup $patcha cmd list_files done done done Aha! That's better, IMO. I'll try that and post a version 2 of the series. It might take a few days, though. No problem. I'm still the slower one of the two of us. :/ Jeff. There were a few details that made it a bit more complex than that, but I think the end result was still an improvement. The most obvious detail is that if you add two empty patches, you cannot name them both empty, so when $patcha and $patchb is the same you have to add suffixes. The other detail is that my tests used different commit messages when both commits contained a message. I want to retain that behaviour, so that added a few lines of complexity. I'll post an updated patch series once I've gone through all your comments. In the meantime, you can see my new implementation here: http://repo.or.cz/w/guilt/ceder.git/commitdiff/3107dc73eaff020da18024c3b5f5f92b94d17852?hp=6df110c95133d6e557ce3dbcb6fd39bc797f877b#patch2 /ceder -- 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: [GUILT 07/28] Added test cases for guilt fold.
On Thu, May 08, 2014 at 09:41:22PM +0200, Per Cederqvist wrote: On Wed, May 7, 2014 at 11:06 PM, Jeff Sipek jef...@josefsipek.net wrote: On Wed, May 07, 2014 at 10:59:56PM +0200, Per Cederqvist wrote: On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek jef...@josefsipek.net wrote: On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote: Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) I feel like we should have *some* content there - most of the time, I care more about the diffs getting folded than the commit message :) I added these tests for a reason: the reproduce a bug in guilt that I found. I'm afraid that having some content might hide the bug I found. (I'm also equally afraid that it might uncover other bugs in guilt, which would delay integration of this patch series. So adding more test cases with content is a good thing to do, but maybe not in this patch series.) Fair enough. I use guilt-fold all the time and it hasn't lost any of my diffs, so I'm happy to defer this until some point in the future. ... for using_diffstat in true false ; do for patcha in empty nonempty ; do for patchb in empty nonempty ; do echo %% $patcha + $patchb (diffstat=$using_diffstat) ${patcha}_patch $patcha ${patchb}_patch $patchb cmd guilt pop $patchb cmd guilt fold $patchb fixup_time_info $patcha cmd list_files cleanup $patcha cmd list_files done done done Aha! That's better, IMO. I'll try that and post a version 2 of the series. It might take a few days, though. No problem. I'm still the slower one of the two of us. :/ Jeff. There were a few details that made it a bit more complex than that, but I think the end result was still an improvement. The devil's in the details :) The most obvious detail is that if you add two empty patches, you cannot name them both empty, so when $patcha and $patchb is the same you have to add suffixes. The other detail is that my tests used different commit messages when both commits contained a message. I want to retain that behaviour, so that added a few lines of complexity. I'll post an updated patch series once I've gone through all your comments. In the meantime, you can see my new implementation here: http://repo.or.cz/w/guilt/ceder.git/commitdiff/3107dc73eaff020da18024c3b5f5f92b94d17852?hp=6df110c95133d6e557ce3dbcb6fd39bc797f877b#patch2 Much better than the original, IMO. Thanks, Jeff. -- We have joy, we have fun, we have Linux on a Sun... -- 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: [GUILT 07/28] Added test cases for guilt fold.
On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek jef...@josefsipek.net wrote: On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote: Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) I feel like we should have *some* content there - most of the time, I care more about the diffs getting folded than the commit message :) I added these tests for a reason: the reproduce a bug in guilt that I found. I'm afraid that having some content might hide the bug I found. (I'm also equally afraid that it might uncover other bugs in guilt, which would delay integration of this patch series. So adding more test cases with content is a good thing to do, but maybe not in this patch series.) Signed-off-by: Per Cederqvist ced...@opera.com --- regression/t-035.out | 659 +++ regression/t-035.sh | 88 +++ 2 files changed, 747 insertions(+) create mode 100644 regression/t-035.out create mode 100755 regression/t-035.sh diff --git a/regression/t-035.out b/regression/t-035.out new file mode 100644 index 000..04af146 --- /dev/null +++ b/regression/t-035.out @@ -0,0 +1,659 @@ +% setup_repo +% git config guilt.diffstat true +%% empty + empty (diffstat=true) +% guilt new empty-1 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% guilt new empty-2 +% guilt pop +Now at empty-1. +% guilt push +Applying patch..empty-2 +Patch applied. +% guilt pop +Now at empty-1. +% guilt fold empty-2 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 4ea806e306f0228a8ef41f186035e7b04097f1f2 .git/patches/master/status +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty-1 +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d28d87b88c1e24d637e390dc3603cfa7c1715711 .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r bde3d337af70f36836ad606c800d194006f883b3 .git/refs/patches/master/empty-1 +% git log -p Strictly speaking, git log isn't necessary since list_files prints the hashes of each of the files as well as the refs for all the applied patches. If anything mismatches, the hashes will catch it. I'm ok with keeping the git log here as long as people can't mess up the formatting with git config/etc. Having the patches included was very helpful while I developed the test case, but your concern about messing up the formatting is valid. I'll remove them. ... diff --git a/regression/t-035.sh b/regression/t-035.sh new file mode 100755 index 000..aed3ef2 --- /dev/null +++ b/regression/t-035.sh @@ -0,0 +1,88 @@ +#!/bin/bash +# +# Test the fold code +# + +source $REG_DIR/scaffold + +cmd setup_repo + +function fixup_time_info +{ + cmd guilt pop + touch -a -m -t $TOUCH_DATE .git/patches/master/$1 + cmd guilt push +} + +function test_fold +{ +using_diffstat=$1 + +cmd git config guilt.diffstat $using_diffstat + +# Empty message + empty message = empty message. +echo %% empty + empty (diffstat=$using_diffstat) +cmd guilt new empty-1 +fixup_time_info empty-1 +cmd guilt new empty-2 +fixup_time_info empty-2 +cmd guilt pop +cmd guilt fold empty-2 +fixup_time_info empty-1 +cmd list_files +cmd git log -p +cmd guilt pop +cmd guilt delete -f empty-1 +cmd list_files + +# Empty message + non-empty message +echo %% empty + non-empty (diffstat=$using_diffstat) +cmd guilt new empty +fixup_time_info empty +cmd echo test a I see these redirected echos... what are they for? The intent was to create files with some content. I was under the impression that the guilt new on the next line would include the file, since I use the -f option. Now I see that that is not the case, and I will remove these lines. +cmd guilt new -f -s -m A commit message. non-empty +fixup_time_info non-empty +cmd guilt pop +cmd guilt fold non-empty +fixup_time_info empty +cmd list_files +cmd git log -p +cmd guilt pop +cmd guilt delete -f empty +cmd list_files Maybe make two helper functions.. one to make a patch with an empty message and one to make a patch with a non-empty message. Then each of these blocks would look a bit cleaner. echo %% empty + non-empty (diffstat=$using_diffstat) empty_patch empty
Re: [GUILT 07/28] Added test cases for guilt fold.
On Wed, May 07, 2014 at 10:59:56PM +0200, Per Cederqvist wrote: On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek jef...@josefsipek.net wrote: On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote: Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) I feel like we should have *some* content there - most of the time, I care more about the diffs getting folded than the commit message :) I added these tests for a reason: the reproduce a bug in guilt that I found. I'm afraid that having some content might hide the bug I found. (I'm also equally afraid that it might uncover other bugs in guilt, which would delay integration of this patch series. So adding more test cases with content is a good thing to do, but maybe not in this patch series.) Fair enough. I use guilt-fold all the time and it hasn't lost any of my diffs, so I'm happy to defer this until some point in the future. ... for using_diffstat in true false ; do for patcha in empty nonempty ; do for patchb in empty nonempty ; do echo %% $patcha + $patchb (diffstat=$using_diffstat) ${patcha}_patch $patcha ${patchb}_patch $patchb cmd guilt pop $patchb cmd guilt fold $patchb fixup_time_info $patcha cmd list_files cleanup $patcha cmd list_files done done done Aha! That's better, IMO. I'll try that and post a version 2 of the series. It might take a few days, though. No problem. I'm still the slower one of the two of us. :/ Jeff. -- Keyboard not found! Press F1 to enter Setup -- 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: [GUILT 07/28] Added test cases for guilt fold.
On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote: Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) I feel like we should have *some* content there - most of the time, I care more about the diffs getting folded than the commit message :) Signed-off-by: Per Cederqvist ced...@opera.com --- regression/t-035.out | 659 +++ regression/t-035.sh | 88 +++ 2 files changed, 747 insertions(+) create mode 100644 regression/t-035.out create mode 100755 regression/t-035.sh diff --git a/regression/t-035.out b/regression/t-035.out new file mode 100644 index 000..04af146 --- /dev/null +++ b/regression/t-035.out @@ -0,0 +1,659 @@ +% setup_repo +% git config guilt.diffstat true +%% empty + empty (diffstat=true) +% guilt new empty-1 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% guilt new empty-2 +% guilt pop +Now at empty-1. +% guilt push +Applying patch..empty-2 +Patch applied. +% guilt pop +Now at empty-1. +% guilt fold empty-2 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 4ea806e306f0228a8ef41f186035e7b04097f1f2 .git/patches/master/status +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty-1 +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d28d87b88c1e24d637e390dc3603cfa7c1715711 .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r bde3d337af70f36836ad606c800d194006f883b3 .git/refs/patches/master/empty-1 +% git log -p Strictly speaking, git log isn't necessary since list_files prints the hashes of each of the files as well as the refs for all the applied patches. If anything mismatches, the hashes will catch it. I'm ok with keeping the git log here as long as people can't mess up the formatting with git config/etc. ... diff --git a/regression/t-035.sh b/regression/t-035.sh new file mode 100755 index 000..aed3ef2 --- /dev/null +++ b/regression/t-035.sh @@ -0,0 +1,88 @@ +#!/bin/bash +# +# Test the fold code +# + +source $REG_DIR/scaffold + +cmd setup_repo + +function fixup_time_info +{ + cmd guilt pop + touch -a -m -t $TOUCH_DATE .git/patches/master/$1 + cmd guilt push +} + +function test_fold +{ +using_diffstat=$1 + +cmd git config guilt.diffstat $using_diffstat + +# Empty message + empty message = empty message. +echo %% empty + empty (diffstat=$using_diffstat) +cmd guilt new empty-1 +fixup_time_info empty-1 +cmd guilt new empty-2 +fixup_time_info empty-2 +cmd guilt pop +cmd guilt fold empty-2 +fixup_time_info empty-1 +cmd list_files +cmd git log -p +cmd guilt pop +cmd guilt delete -f empty-1 +cmd list_files + +# Empty message + non-empty message +echo %% empty + non-empty (diffstat=$using_diffstat) +cmd guilt new empty +fixup_time_info empty +cmd echo test a I see these redirected echos... what are they for? +cmd guilt new -f -s -m A commit message. non-empty +fixup_time_info non-empty +cmd guilt pop +cmd guilt fold non-empty +fixup_time_info empty +cmd list_files +cmd git log -p +cmd guilt pop +cmd guilt delete -f empty +cmd list_files Maybe make two helper functions.. one to make a patch with an empty message and one to make a patch with a non-empty message. Then each of these blocks would look a bit cleaner. echo %% empty + non-empty (diffstat=$using_diffstat) empty_patch empty nonempty_patch non-empty cmd guilt pop non-empty cmd guilt fold non-empty fixup_time_info empty cmd list_files cleanup empty cmd list_files cleanup() { guilt pop $1 guilt delete -f $1 } Eh, it's not as clean as I thought it would be, but I think it's still a bit better. Ok, how about: for using_diffstat in true false ; do for patcha in empty nonempty ; do for patchb in empty nonempty ; do echo %% $patcha + $patchb (diffstat=$using_diffstat) ${patcha}_patch $patcha ${patchb}_patch $patchb cmd guilt pop $patchb cmd guilt fold $patchb fixup_time_info $patcha cmd
[GUILT 07/28] Added test cases for guilt fold.
Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) Signed-off-by: Per Cederqvist ced...@opera.com --- regression/t-035.out | 659 +++ regression/t-035.sh | 88 +++ 2 files changed, 747 insertions(+) create mode 100644 regression/t-035.out create mode 100755 regression/t-035.sh diff --git a/regression/t-035.out b/regression/t-035.out new file mode 100644 index 000..04af146 --- /dev/null +++ b/regression/t-035.out @@ -0,0 +1,659 @@ +% setup_repo +% git config guilt.diffstat true +%% empty + empty (diffstat=true) +% guilt new empty-1 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% guilt new empty-2 +% guilt pop +Now at empty-1. +% guilt push +Applying patch..empty-2 +Patch applied. +% guilt pop +Now at empty-1. +% guilt fold empty-2 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 4ea806e306f0228a8ef41f186035e7b04097f1f2 .git/patches/master/status +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty-1 +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d28d87b88c1e24d637e390dc3603cfa7c1715711 .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r bde3d337af70f36836ad606c800d194006f883b3 .git/refs/patches/master/empty-1 +% git log -p +commit bde3d337af70f36836ad606c800d194006f883b3 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +patch empty-1 + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% guilt pop +All patches popped. +% guilt delete -f empty-1 +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +%% empty + non-empty (diffstat=true) +% guilt new empty +% guilt pop +All patches popped. +% guilt push +Applying patch..empty +Patch applied. +% guilt new -f -s -m A commit message. non-empty +% guilt pop +Now at empty. +% guilt push +Applying patch..non-empty +Patch applied. +% guilt pop +Now at empty. +% guilt fold non-empty +% guilt pop +All patches popped. +% guilt push +Applying patch..empty +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 15aab0fd8b937eb3bb01841693f35dcb75da2faf .git/patches/master/status +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/empty~ +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/non-empty~ +f 683678040eef9334d6329e00d5b9babda3e65b57 .git/patches/master/empty +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f a26a22287b500a2a372e42c2bab03599bbe37cdf .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r 4eedaa32894fc07af3298d8c1178052942a3ca6a .git/refs/patches/master/empty +% git log -p +commit 4eedaa32894fc07af3298d8c1178052942a3ca6a +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +A commit message. + +Signed-off-by: Commiter Name commiter@email + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +%