[PATCH v2 0/5] t4214: cleanup and demonstrate graph bug
To
Git Mailing List
Cc
Allan Caffee
Noam Postavsky
Jeff King
Junio C Hamano
From
Denton Liu
See Also
Prev
Date
2019-10-04 00:23:09 UTC
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