Junio, the test cases from earlier didn't exactly cover the cases Peff
talked about so I added a few more test cases. These should cover those
situations and a few more so we can be extra sure when the bug is fixed.


This patchset cleans up t4214 and then, in the last patch, demonstrates
a bug in the way some octopus merges are colored.

While I was playing around with Pratyush's octopus merge in git-gui, I
noticed that there was a bug in `git log --graph`. The horizontal lines
in the octopus merge seemed to have an off-by-one error in their
coloring. More detail in the last patch.

I tried my hand at fixing the bug but in the hour I spent going at it, I
couldn't fix the logic up. The buggy logic is in graph.c:
graph_draw_octopus_merge() in case anyone is interested.

Changes since v1:

* Add a few more test cases to demonstrate more failure (and passing)
  conditions


Denton Liu (5):
  test-lib: let test_merge() perform octopus merges
  t4214: use test_merge
  t4214: generate expect in their own test cases
  t4214: explicitly list tags in log
  t4214: demonstrate octopus graph coloring failure

 t/t4214-log-graph-octopus.sh | 329 ++++++++++++++++++++++++++++++++---
 t/test-lib-functions.sh      |   6 +-
 2 files changed, 308 insertions(+), 27 deletions(-)

Range-diff against v1:
1:  e77af8cde5 = 1:  e77af8cde5 test-lib: let test_merge() perform octopus 
merges
2:  4a93deb3f6 = 2:  4a93deb3f6 t4214: use test_merge
3:  c09f761185 = 3:  c09f761185 t4214: generate expect in their own test cases
4:  ad6d89440b = 4:  ad6d89440b t4214: explicitly list tags in log
5:  0b84bf5417 ! 5:  e58c1929bc t4214: demonstrate octopus graph coloring 
failure
    @@ Commit message
         dash should be the color of the line to their bottom-right. Instead, 
they
         are currently the color of the line to their bottom.
     
    -    Demonstrate this breakage with two sets of test cases. The first pair 
of
    -    test cases demonstrates the breakage with a similar case as the above.
    -    The second pair of test cases demonstrates a similar breakage but with
    -    the last parent crossing over.
    +    Demonstrate this breakage with a few sets of test cases. These test
    +    cases should show not only simple cases of the bug occuring but 
trickier
    +    situations that may not be handled properly in any attempt to fix the
    +    bug.
     
    -    The second pair of test cases are included as a result of my (poor)
    -    attempts at fixing the bug. This case seems particularly tricky to
    -    handle. Good luck!
    +    While we're at it, include a passing test case as a canary in case an
    +    attempt to fix the bug breaks existing operation.
     
      ## t/t4214-log-graph-octopus.sh ##
     @@ t/t4214-log-graph-octopus.sh: test_expect_success 'set up merge 
history' '
    @@ t/t4214-log-graph-octopus.sh: test_expect_success 'set up merge history' 
'
     -  test_commit left
     +  test_commit left &&
     +  git checkout 4 -b crossover &&
    -+  test_commit after-4
    ++  test_commit after-4 &&
    ++  git checkout initial -b more-L &&
    ++  test_commit after-initial
      '
      
      test_expect_success 'log --graph with tricky octopus merge, no color' '
    @@ t/t4214-log-graph-octopus.sh: test_expect_success 'log --graph with 
normal octop
        test_cmp expect.colors actual.colors
      '
     +
    -+test_expect_success 'log --graph with tricky octopus merge and its 
parent, no color' '
    ++test_expect_success 'log --graph with normal octopus merge and child, no 
color' '
    ++  cat >expect.uncolored <<-\EOF &&
    ++  * after-merge
    ++  *---.   octopus-merge
    ++  |\ \ \
    ++  | | | * 4
    ++  | | * | 3
    ++  | | |/
    ++  | * | 2
    ++  | |/
    ++  * | 1
    ++  |/
    ++  * initial
    ++  EOF
    ++  git log --color=never --graph --date-order --pretty=tformat:%s 
after-merge >actual.raw &&
    ++  sed "s/ *\$//" actual.raw >actual &&
    ++  test_cmp expect.uncolored actual
    ++'
    ++
    ++test_expect_failure 'log --graph with normal octopus and child merge with 
colors' '
    ++  cat >expect.colors <<-\EOF &&
    ++  * after-merge
    ++  *<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET>   
octopus-merge
    ++  <GREEN>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> <MAGENTA>\<RESET>
    ++  <GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> * 4
    ++  <GREEN>|<RESET> <YELLOW>|<RESET> * <MAGENTA>|<RESET> 3
    ++  <GREEN>|<RESET> <YELLOW>|<RESET> <MAGENTA>|<RESET><MAGENTA>/<RESET>
    ++  <GREEN>|<RESET> * <MAGENTA>|<RESET> 2
    ++  <GREEN>|<RESET> <MAGENTA>|<RESET><MAGENTA>/<RESET>
    ++  * <MAGENTA>|<RESET> 1
    ++  <MAGENTA>|<RESET><MAGENTA>/<RESET>
    ++  * initial
    ++  EOF
    ++  test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
    ++  git log --color=always --graph --date-order --pretty=tformat:%s 
after-merge >actual.colors.raw &&
    ++  test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
    ++  test_cmp expect.colors actual.colors
    ++'
    ++
    ++test_expect_success 'log --graph with tricky octopus merge and its child, 
no color' '
     +  cat >expect.uncolored <<-\EOF &&
     +  * left
     +  | * after-merge
    @@ t/t4214-log-graph-octopus.sh: test_expect_success 'log --graph with 
normal octop
     +  test_cmp expect.uncolored actual
     +'
     +
    -+test_expect_failure 'log --graph with tricky octopus merge and its parent 
with colors' '
    ++test_expect_failure 'log --graph with tricky octopus merge and its child 
with colors' '
     +  test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
     +  cat >expect.colors <<-\EOF &&
     +  * left
    @@ t/t4214-log-graph-octopus.sh: test_expect_success 'log --graph with 
normal octop
     +  test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
     +  test_cmp expect.colors actual.colors
     +'
    ++
    ++test_expect_success 'log --graph with crossover in octopus merge and its 
child, no color' '
    ++  cat >expect.uncolored <<-\EOF &&
    ++  * after-4
    ++  | * after-merge
    ++  | *---.   octopus-merge
    ++  | |\ \ \
    ++  | |_|_|/
    ++  |/| | |
    ++  * | | | 4
    ++  | | | * 3
    ++  | |_|/
    ++  |/| |
    ++  | | * 2
    ++  | |/
    ++  |/|
    ++  | * 1
    ++  |/
    ++  * initial
    ++  EOF
    ++  git log --color=never --graph --date-order --pretty=tformat:%s after-4 
after-merge >actual.raw &&
    ++  sed "s/ *\$//" actual.raw >actual &&
    ++  test_cmp expect.uncolored actual
    ++'
    ++
    ++test_expect_failure 'log --graph with crossover in octopus merge and its 
child with colors' '
    ++  test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
    ++  cat >expect.colors <<-\EOF &&
    ++  * after-4
    ++  <RED>|<RESET> * after-merge
    ++  <RED>|<RESET> 
*<MAGENTA>-<RESET><MAGENTA>-<RESET><RED>-<RESET><RED>.<RESET>   octopus-merge
    ++  <RED>|<RESET> <YELLOW>|<RESET><BLUE>\<RESET> <MAGENTA>\<RESET> 
<RED>\<RESET>
    ++  <RED>|<RESET> 
<YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>_<RESET><MAGENTA>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><YELLOW>|<RESET> <BLUE>|<RESET> 
<MAGENTA>|<RESET>
    ++  * <YELLOW>|<RESET> <BLUE>|<RESET> <MAGENTA>|<RESET> 4
    ++  <CYAN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> * 3
    ++  <CYAN>|<RESET> 
<YELLOW>|<RESET><CYAN>_<RESET><BLUE>|<RESET><CYAN>/<RESET>
    ++  <CYAN>|<RESET><CYAN>/<RESET><YELLOW>|<RESET> <BLUE>|<RESET>
    ++  <CYAN>|<RESET> <YELLOW>|<RESET> * 2
    ++  <CYAN>|<RESET> <YELLOW>|<RESET><CYAN>/<RESET>
    ++  <CYAN>|<RESET><CYAN>/<RESET><YELLOW>|<RESET>
    ++  <CYAN>|<RESET> * 1
    ++  <CYAN>|<RESET><CYAN>/<RESET>
    ++  * initial
    ++  EOF
    ++  git log --color=always --graph --date-order --pretty=tformat:%s after-4 
after-merge >actual.colors.raw &&
    ++  test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
    ++  test_cmp expect.colors actual.colors
    ++'
    ++
    ++test_expect_success 'log --graph with unrelated commit and octopus tip, 
no color' '
    ++  cat >expect.uncolored <<-\EOF &&
    ++  * after-initial
    ++  | *---.   octopus-merge
    ++  | |\ \ \
    ++  | | | | * 4
    ++  | |_|_|/
    ++  |/| | |
    ++  | | | * 3
    ++  | |_|/
    ++  |/| |
    ++  | | * 2
    ++  | |/
    ++  |/|
    ++  | * 1
    ++  |/
    ++  * initial
    ++  EOF
    ++  git log --color=never --graph --date-order --pretty=tformat:%s 
after-initial octopus-merge >actual.raw &&
    ++  sed "s/ *\$//" actual.raw >actual &&
    ++  test_cmp expect.uncolored actual
    ++'
    ++
    ++test_expect_success 'log --graph with unrelated commit and octopus tip 
with colors' '
    ++  test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
    ++  cat >expect.colors <<-\EOF &&
    ++  * after-initial
    ++  <RED>|<RESET> 
*<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET>   octopus-merge
    ++  <RED>|<RESET> <GREEN>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> 
<MAGENTA>\<RESET>
    ++  <RED>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> * 4
    ++  <RED>|<RESET> 
<GREEN>|<RESET><RED>_<RESET><YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET> 
<BLUE>|<RESET>
    ++  <RED>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> * 3
    ++  <RED>|<RESET> <GREEN>|<RESET><RED>_<RESET><YELLOW>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET>
    ++  <RED>|<RESET> <GREEN>|<RESET> * 2
    ++  <RED>|<RESET> <GREEN>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><GREEN>|<RESET>
    ++  <RED>|<RESET> * 1
    ++  <RED>|<RESET><RED>/<RESET>
    ++  * initial
    ++  EOF
    ++  git log --color=always --graph --date-order --pretty=tformat:%s 
after-initial octopus-merge >actual.colors.raw &&
    ++  test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
    ++  test_cmp expect.colors actual.colors
    ++'
    ++
    ++test_expect_success 'log --graph with unrelated commit and octopus child, 
no color' '
    ++  cat >expect.uncolored <<-\EOF &&
    ++  * after-initial
    ++  | * after-merge
    ++  | *---.   octopus-merge
    ++  | |\ \ \
    ++  | | | | * 4
    ++  | |_|_|/
    ++  |/| | |
    ++  | | | * 3
    ++  | |_|/
    ++  |/| |
    ++  | | * 2
    ++  | |/
    ++  |/|
    ++  | * 1
    ++  |/
    ++  * initial
    ++  EOF
    ++  git log --color=never --graph --date-order --pretty=tformat:%s 
after-initial after-merge >actual.raw &&
    ++  sed "s/ *\$//" actual.raw >actual &&
    ++  test_cmp expect.uncolored actual
    ++'
    ++
    ++test_expect_failure 'log --graph with unrelated commit and octopus child 
with colors' '
    ++  test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
    ++  cat >expect.colors <<-\EOF &&
    ++  * after-initial
    ++  <RED>|<RESET> * after-merge
    ++  <RED>|<RESET> 
*<MAGENTA>-<RESET><MAGENTA>-<RESET><CYAN>-<RESET><CYAN>.<RESET>   octopus-merge
    ++  <RED>|<RESET> <YELLOW>|<RESET><BLUE>\<RESET> <MAGENTA>\<RESET> 
<CYAN>\<RESET>
    ++  <RED>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> <MAGENTA>|<RESET> * 4
    ++  <RED>|<RESET> 
<YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>_<RESET><MAGENTA>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><YELLOW>|<RESET> <BLUE>|<RESET> 
<MAGENTA>|<RESET>
    ++  <RED>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> * 3
    ++  <RED>|<RESET> <YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><YELLOW>|<RESET> <BLUE>|<RESET>
    ++  <RED>|<RESET> <YELLOW>|<RESET> * 2
    ++  <RED>|<RESET> <YELLOW>|<RESET><RED>/<RESET>
    ++  <RED>|<RESET><RED>/<RESET><YELLOW>|<RESET>
    ++  <RED>|<RESET> * 1
    ++  <RED>|<RESET><RED>/<RESET>
    ++  * initial
    ++  EOF
    ++  git log --color=always --graph --date-order --pretty=tformat:%s 
after-initial after-merge >actual.colors.raw &&
    ++  test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
    ++  test_cmp expect.colors actual.colors
    ++'
     +
      test_done
-- 
2.23.0.565.g1cc52d20df

Reply via email to