On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek <jeffpc@xxxxxxxxxxxxxx> 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 <cederp@xxxxxxxxx>
>> ---
>> 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 0000000..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 0000000..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
> 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 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.
> Adding the non-empty diff is a matter of another loop, but I'm ok with just
> testing the headers (for now) :)
>
> Jeff.
>
/ceder
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html