Re: [GUILT 07/28] Added test cases for "guilt fold".

On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek [off-list ref] wrote:

On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote:

quoted

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.)

quoted

Signed-off-by: Per Cederqvist <redacted>
---
 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.

...

quoted

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.

quoted

+    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