git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Per Cederqvist <cederp@opera.com>
Date
May 7, 2014, 20:59 UTC
Message-ID
<CAP=KgsQ2fvVWZG7OwwWac+tMeWaPTEdSrgFqE5gVfSAqNn-VCQ@mail.gmail.com>
In-Reply-To
<20140506194026.GK1655@meili.valhalla.31bits.net>
On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek <jeffpc@josefsipek.net> wrote:
Show 7 quoted lines
> 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.)

Show 59 quoted lines
>> Signed-off-by: Per Cederqvist <cederp@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 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.

Show 51 quoted lines
> ...
>> 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.

Show 51 quoted 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.

Show 5 quoted lines
> Adding the non-empty diff is a matter of another loop, but I'm ok with just
> testing the headers (for now) :)
>
> Jeff.
>
    /ceder
Previous: Jeff SipekNext: Jeff Sipek
Message 19 of 61 in “[GUILT 00/28] Teach guilt import-commit how create legal patch names, and more”
  1. Per CederqvistMar 21, 2014
  2. 01/28 The tests should not fail if guilt.diffstat is set.Per Cederqvist, Mar 21, 2014
  3. 02/28 Allow "guilt delete -f" to run from a dir which contains spaces.Per Cederqvist, Mar 21, 2014
  4. 03/28 Added test case for "guilt delete -f".Per Cederqvist, Mar 21, 2014
  5. 04/28 Allow "guilt import-commit" to run from a dir which contains spaces.Per Cederqvist, Mar 21, 2014
  6. Jeff SipekMar 23, 2014
  7. Per CederqvistMar 23, 2014
  8. Jeff SipekMar 23, 2014
  9. Per CederqvistMar 23, 2014
  10. Jeff SipekMay 6, 2014
  11. 05/28 "guilt new": Accept more than 4 arguments.Per Cederqvist, Mar 21, 2014
  12. 06/28 Fix and simplify the do_get_patch function.Per Cederqvist, Mar 21, 2014
  13. Jeff SipekMar 23, 2014
  14. Per CederqvistMar 23, 2014
  15. Jeff SipekMay 6, 2014
  16. Per CederqvistMay 7, 2014
  17. 07/28 Added test cases for "guilt fold".Per Cederqvist, Mar 21, 2014
  18. Jeff SipekMay 6, 2014
  19. Per CederqvistMay 7, 2014
  20. Jeff SipekMay 7, 2014
  21. Per CederqvistMay 8, 2014
  22. Jeff SipekMay 8, 2014
  23. 08/28 Added more test cases for "guilt new": empty patches.Per Cederqvist, Mar 21, 2014
  24. 09/28 Test suite: properly check the exit status of commands.Per Cederqvist, Mar 21, 2014
  25. 10/28 Run test_failed if the exit status of a test script is bad.Per Cederqvist, Mar 21, 2014
  26. Jeff SipekMay 6, 2014
  27. 11/28 test suite: remove pointless redirection.Per Cederqvist, Mar 21, 2014
  28. Jeff SipekMay 6, 2014
  29. 12/28 "guilt header": more robust header selection.Per Cederqvist, Mar 21, 2014
  30. 13/28 Check that "guilt header '.*'" fails.Per Cederqvist, Mar 21, 2014
  31. 14/28 Use "git check-ref-format" to validate patch names.Per Cederqvist, Mar 21, 2014
  32. 15/28 Produce legal patch names in guilt-import-commit.Per Cederqvist, Mar 21, 2014
  33. 16/28 Fix backslash handling when creating names of imported patches.Per Cederqvist, Mar 21, 2014
  34. 17/28 "guilt graph" no longer loops when no patches are applied.Per Cederqvist, Mar 21, 2014
  35. Jeff SipekMay 6, 2014
  36. 18/28 guilt-graph: Handle commas in branch names.Per Cederqvist, Mar 21, 2014
  37. Jeff SipekMay 6, 2014
  38. 19/28 Check that "guilt graph" works when working on a branch with a comma.Per Cederqvist, Mar 21, 2014
  39. Jeff SipekMay 6, 2014
  40. Jeff SipekMay 13, 2014
  41. Jeff SipekMay 13, 2014
  42. 20/28 "guilt graph": Handle patch names containing quotes.Per Cederqvist, Mar 21, 2014
  43. Eric SunshineMar 21, 2014
  44. Jeff SipekMay 6, 2014
  45. Per CederqvistMay 8, 2014
  46. 21/28 The log.decorate setting should not influence import-commit.Per Cederqvist, Mar 21, 2014
  47. Jeff SipekMay 6, 2014
  48. 22/28 The log.decorate setting should not influence patchbomb.Per Cederqvist, Mar 21, 2014
  49. Jeff SipekMay 6, 2014
  50. 23/28 The log.decorate setting should not influence guilt rebase.Per Cederqvist, Mar 21, 2014
  51. Jeff SipekMay 6, 2014
  52. 24/28 disp no longer processes backslashes.Per Cederqvist, Mar 21, 2014
  53. Jeff SipekMay 7, 2014
  54. 25/28 "guilt push" now fails when there are no more patches to push.Per Cederqvist, Mar 21, 2014
  55. Jeff SipekMay 7, 2014
  56. Per CederqvistMay 8, 2014
  57. 26/28 "guilt pop" now fails when there are no more patches to pop.Per Cederqvist, Mar 21, 2014
  58. 27/28 Minor testsuite fix.Per Cederqvist, Mar 21, 2014
  59. Jeff SipekMay 7, 2014
  60. 28/28 Added guilt.reusebranch configuration option.Per Cederqvist, Mar 21, 2014
  61. Jeff SipekMar 21, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.