[BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug

[BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug

[Denton Liu]

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.


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 | 143 +++++++++++++++++++++++++++++------
 t/test-lib-functions.sh      |   6 +-
 2 files changed, 122 insertions(+), 27 deletions(-)

-- 
2.23.0.248.g3a9dd8fb08

[BUG/PATCH 1/5] test-lib: let test_merge() perform octopus merges

[Denton Liu]

Currently test_merge() only allows developers to merge in one branch.
However, this restriction is artificial and there is no reason why it
needs to be this way.

Extend test_merge() to allow the specification of multiple branches so
that octopus merges can be performed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/test-lib-functions.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 87bf3a2287..b299ecc326 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -228,9 +228,11 @@ test_commit () {
 # can be a tag pointing to the commit-to-merge.
 
 test_merge () {
+	label="$1" &&
+	shift &&
 	test_tick &&
-	git merge -m "$1" "$2" &&
-	git tag "$1"
+	git merge -m "$label" "$@" &&
+	git tag "$label"
 }
 
 # Efficiently create <nr> commits, each with a unique number (from 1 to <nr>
-- 
2.23.0.248.g3a9dd8fb08

[BUG/PATCH 2/5] t4214: use test_merge

[Denton Liu]

In the previous commit, we extended test_merge() so that it could
perform octopus merges. Now that the restriction is lifted, use
test_merge() to perform the octopus merge instead of manually
duplicating test_merge() functionality.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index dab96c89aa..f6e22ec825 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -41,8 +41,7 @@ test_expect_success 'set up merge history' '
 		test_commit $i $i $i tag$i || return $?
 	done &&
 	git checkout 1 -b merge &&
-	test_tick &&
-	git merge -m octopus-merge 1 2 3 4 &&
+	test_merge octopus-merge 1 2 3 4 &&
 	git checkout 1 -b L &&
 	test_commit left
 '
-- 
2.23.0.248.g3a9dd8fb08

[BUG/PATCH 3/5] t4214: generate expect in their own test cases

[Denton Liu]

Before, the expect files of the test case were being generated in the
setup method. However, it would make more sense to generate these files
within the test cases that actually use them so that it's obvious to
future readers where the expected values are coming from.

Move the generation of the expect files in their own respective test
cases.

While we're at it, we want to establish a pattern in this test suite
that, firstly, a non-colored test case is given then, immediately after,
the colored version is given.

Switch test cases "log --graph with tricky octopus merge, no color" and
"log --graph with tricky octopus merge with colors" so that the "no
color" version appears first.

This patch is best viewed with `--color-moved`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 42 ++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index f6e22ec825..16776e347c 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -5,6 +5,20 @@ test_description='git log --graph of skewed left octopus merge.'
 . ./test-lib.sh
 
 test_expect_success 'set up merge history' '
+	test_commit initial &&
+	for i in 1 2 3 4 ; do
+		git checkout master -b $i || return $?
+		# Make tag name different from branch name, to avoid
+		# ambiguity error when calling checkout.
+		test_commit $i $i $i tag$i || return $?
+	done &&
+	git checkout 1 -b merge &&
+	test_merge octopus-merge 1 2 3 4 &&
+	git checkout 1 -b L &&
+	test_commit left
+'
+
+test_expect_success 'log --graph with tricky octopus merge, no color' '
 	cat >expect.uncolored <<-\EOF &&
 	* left
 	| *---.   octopus-merge
@@ -19,6 +33,13 @@ test_expect_success 'set up merge history' '
 	|/
 	* initial
 	EOF
+	git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
+	sed "s/ *\$//" actual.raw >actual &&
+	test_cmp expect.uncolored actual
+'
+
+test_expect_success 'log --graph with tricky octopus merge with colors' '
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
 	cat >expect.colors <<-\EOF &&
 	* left
 	<RED>|<RESET> *<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET>   octopus-merge
@@ -33,32 +54,11 @@ test_expect_success 'set up merge history' '
 	<MAGENTA>|<RESET><MAGENTA>/<RESET>
 	* initial
 	EOF
-	test_commit initial &&
-	for i in 1 2 3 4 ; do
-		git checkout master -b $i || return $?
-		# Make tag name different from branch name, to avoid
-		# ambiguity error when calling checkout.
-		test_commit $i $i $i tag$i || return $?
-	done &&
-	git checkout 1 -b merge &&
-	test_merge octopus-merge 1 2 3 4 &&
-	git checkout 1 -b L &&
-	test_commit left
-'
-
-test_expect_success 'log --graph with tricky octopus merge with colors' '
-	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
 	git log --color=always --graph --date-order --pretty=tformat:%s --all >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, no color' '
-	git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
-	sed "s/ *\$//" actual.raw >actual &&
-	test_cmp expect.uncolored actual
-'
-
 # Repeat the previous two tests with "normal" octopus merge (i.e.,
 # without the first parent skewing to the "left" branch column).
 
-- 
2.23.0.248.g3a9dd8fb08

[BUG/PATCH 4/5] t4214: explicitly list tags in log

[Denton Liu]

In a future test case, we will be extending the commit graph. As a
result, explicitly list the tags that will generate the graph so that
when future additions are made, the current graph illustrations won't be
affected.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 16776e347c..097151da39 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -33,7 +33,7 @@ test_expect_success 'log --graph with tricky octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
+	git log --color=never --graph --date-order --pretty=tformat:%s left octopus-merge >actual.raw &&
 	sed "s/ *\$//" actual.raw >actual &&
 	test_cmp expect.uncolored actual
 '
@@ -54,7 +54,7 @@ test_expect_success 'log --graph with tricky octopus merge with colors' '
 	<MAGENTA>|<RESET><MAGENTA>/<RESET>
 	* initial
 	EOF
-	git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw &&
+	git log --color=always --graph --date-order --pretty=tformat:%s left octopus-merge >actual.colors.raw &&
 	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
 	test_cmp expect.colors actual.colors
 '
@@ -75,7 +75,7 @@ test_expect_success 'log --graph with normal octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s merge >actual.raw &&
+	git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw &&
 	sed "s/ *\$//" actual.raw >actual &&
 	test_cmp expect.uncolored actual
 '
@@ -94,7 +94,7 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 	* initial
 	EOF
 	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
-	git log --color=always --graph --date-order --pretty=tformat:%s merge >actual.colors.raw &&
+	git log --color=always --graph --date-order --pretty=tformat:%s octopus-merge >actual.colors.raw &&
 	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
 	test_cmp expect.colors actual.colors
 '
-- 
2.23.0.248.g3a9dd8fb08

[BUG/PATCH 5/5] t4214: demonstrate octopus graph coloring failure

[Denton Liu]

The graph coloring logic for octopus merges currently has a bug. This
can be seen git.git with 74c7cfa875 (Merge of
http://members.cox.net/junkio/git-jc.git, 2005-05-05), whose second
child is 211232bae6 (Octopus merge of the following five patches.,
2005-05-05).

If one runs

	git log --graph 74c7cfa875

one can see that the octopus merge is colored incorrectly. In
particular, the horizontal dashes are off by one color. Each horizontal
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.

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!

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 96 +++++++++++++++++++++++++++++++++++-
 1 file changed, 95 insertions(+), 1 deletion(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 097151da39..99e0ea034e 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -14,8 +14,11 @@ test_expect_success 'set up merge history' '
 	done &&
 	git checkout 1 -b merge &&
 	test_merge octopus-merge 1 2 3 4 &&
+	test_commit after-merge &&
 	git checkout 1 -b L &&
-	test_commit left
+	test_commit left &&
+	git checkout 4 -b crossover &&
+	test_commit after-4
 '
 
 test_expect_success 'log --graph with tricky octopus merge, no color' '
@@ -98,4 +101,95 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 	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 parent, no color' '
+	cat >expect.uncolored <<-\EOF &&
+	* left
+	| * after-merge
+	| *---.   octopus-merge
+	| |\ \ \
+	|/ / / /
+	| | | * 4
+	| | * | 3
+	| | |/
+	| * | 2
+	| |/
+	* | 1
+	|/
+	* initial
+	EOF
+	git log --color=never --graph --date-order --pretty=tformat:%s left after-merge >actual.raw &&
+	sed "s/ *\$//" actual.raw >actual &&
+	test_cmp expect.uncolored actual
+'
+
+test_expect_failure 'log --graph with tricky octopus merge and its parent with colors' '
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
+	cat >expect.colors <<-\EOF &&
+	* left
+	<RED>|<RESET> * after-merge
+	<RED>|<RESET> *<MAGENTA>-<RESET><MAGENTA>-<RESET><CYAN>-<RESET><CYAN>.<RESET>   octopus-merge
+	<RED>|<RESET> <RED>|<RESET><BLUE>\<RESET> <MAGENTA>\<RESET> <CYAN>\<RESET>
+	<RED>|<RESET><RED>/<RESET> <BLUE>/<RESET> <MAGENTA>/<RESET> <CYAN>/<RESET>
+	<RED>|<RESET> <BLUE>|<RESET> <MAGENTA>|<RESET> * 4
+	<RED>|<RESET> <BLUE>|<RESET> * <CYAN>|<RESET> 3
+	<RED>|<RESET> <BLUE>|<RESET> <CYAN>|<RESET><CYAN>/<RESET>
+	<RED>|<RESET> * <CYAN>|<RESET> 2
+	<RED>|<RESET> <CYAN>|<RESET><CYAN>/<RESET>
+	* <CYAN>|<RESET> 1
+	<CYAN>|<RESET><CYAN>/<RESET>
+	* initial
+	EOF
+	git log --color=always --graph --date-order --pretty=tformat:%s left 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 crossover in octopus merge, no color' '
+	cat >expect.uncolored <<-\EOF &&
+	* after-4
+	| *---.   octopus-merge
+	| |\ \ \
+	| |_|_|/
+	|/| | |
+	* | | | 4
+	| | | * 3
+	| |_|/
+	|/| |
+	| | * 2
+	| |/
+	|/|
+	| * 1
+	|/
+	* initial
+	EOF
+	git log --color=never --graph --date-order --pretty=tformat:%s after-4 octopus-merge >actual.raw &&
+	sed "s/ *\$//" actual.raw >actual &&
+	test_cmp expect.uncolored actual
+'
+
+test_expect_failure 'log --graph with crossover in octopus merge with colors' '
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
+	cat >expect.colors <<-\EOF &&
+	* after-4
+	<RED>|<RESET> *<BLUE>-<RESET><BLUE>-<RESET><RED>-<RESET><RED>.<RESET>   octopus-merge
+	<RED>|<RESET> <GREEN>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> <RED>\<RESET>
+	<RED>|<RESET> <GREEN>|<RESET><RED>_<RESET><YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>/<RESET>
+	<RED>|<RESET><RED>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET>
+	* <GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> 4
+	<MAGENTA>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> * 3
+	<MAGENTA>|<RESET> <GREEN>|<RESET><MAGENTA>_<RESET><YELLOW>|<RESET><MAGENTA>/<RESET>
+	<MAGENTA>|<RESET><MAGENTA>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET>
+	<MAGENTA>|<RESET> <GREEN>|<RESET> * 2
+	<MAGENTA>|<RESET> <GREEN>|<RESET><MAGENTA>/<RESET>
+	<MAGENTA>|<RESET><MAGENTA>/<RESET><GREEN>|<RESET>
+	<MAGENTA>|<RESET> * 1
+	<MAGENTA>|<RESET><MAGENTA>/<RESET>
+	* initial
+	EOF
+	git log --color=always --graph --date-order --pretty=tformat:%s after-4 octopus-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.248.g3a9dd8fb08

Re: [BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug

[Denton Liu]

On Wed, Sep 25, 2019 at 03:26:57AM -0700, Denton Liu wrote:
> 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.

I guess for the record, this was the final patch that I ended up with.
Two issues with it, though: 

1. It assumes that there can be no parallel paths on the right side,
which I'm not sure is a correct assumption.

2. It _still_ fails my last proposed test case.

-- >8 --

diff --git a/graph.c b/graph.c
index f53135485f..f9395a2327 100644
--- a/graph.c
+++ b/graph.c
@@ -881,8 +881,7 @@ static int graph_draw_octopus_merge(struct git_graph *graph,
 	/*
 	 * In both cases, commit_index corresponds to the edge labeled "0".
 	 */
-	int first_col = graph->commit_index + dashless_parents
-	    - parent_in_old_cols;
+	int first_col = graph->num_new_columns - dashful_parents;
 
 	int i;
 	for (i = 0; i < dashful_parents; i++) {

Re: [BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug

[Jeff King]

On Wed, Sep 25, 2019 at 10:09:02AM -0700, Denton Liu wrote:

> On Wed, Sep 25, 2019 at 03:26:57AM -0700, Denton Liu wrote:
> > 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.
> 
> I guess for the record, this was the final patch that I ended up with.
> Two issues with it, though: 
> 
> 1. It assumes that there can be no parallel paths on the right side,
> which I'm not sure is a correct assumption.
> 
> 2. It _still_ fails my last proposed test case.
> 
> -- >8 --
> 
> diff --git a/graph.c b/graph.c
> index f53135485f..f9395a2327 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -881,8 +881,7 @@ static int graph_draw_octopus_merge(struct git_graph *graph,
>  	/*
>  	 * In both cases, commit_index corresponds to the edge labeled "0".
>  	 */
> -	int first_col = graph->commit_index + dashless_parents
> -	    - parent_in_old_cols;
> +	int first_col = graph->num_new_columns - dashful_parents;
>  
>  	int i;
>  	for (i = 0; i < dashful_parents; i++) {

Hmm. Looking at the broken case from "git log -2 --graph 74c7cfa875", I
see that added_cols is "4", which is right (we had 2 lines coming in,
and then with the 5 parents that becomes 6). But one of those lines is
already counted as "dashless". We account for that with
parent_in_old_cols, which is 1, and subtract that way from first_col.
That works for the diagram in the code:

           | *---.
           | |\ \ \
           |/ / / /
           x 0 1 2

where one of the parent lines is collapsing back to the left. But not
for this more mundane case:

  | *-----.   commit 211232bae64bcc60bbf5d1b5e5b2344c22ed767e
  | |\ \ \ \  Merge: fc54a9c30c 9e30dd7c0e c4b83e618f 660265909f b28858bf65
  | | | | | | 

where we go straight down. I'm not sure I've fully grasped it, but it
feels like that distinction is the source of the off-by-one. I'm not
sure how to tell the difference here, though. I think it relies on the
next commit on the left-hand line being the same as the first parent (or
maybe any parent?).

If I remove the use of parent_in_old_cols entirely, the merge looks
right, but the "collapsing" one is broken (and t4214 fails).

By the way, a useful trick I stumbled on to look at the coloring across
many such merges:

  git log --graph --format=%h --color | grep -A2 -e - | less -S

It looks like every octopus in git.git is colored wrong (because they're
the non-collapsing type).

-Peff

Re: [BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> That works for the diagram in the code:
>
>            | *---.
>            | |\ \ \
>            |/ / / /
>            x 0 1 2
>
> where one of the parent lines is collapsing back to the left. But not
> for this more mundane case:
>
>   | *-----.   commit 211232bae64bcc60bbf5d1b5e5b2344c22ed767e
>   | |\ \ \ \  Merge: fc54a9c30c 9e30dd7c0e c4b83e618f 660265909f b28858bf65
>   | | | | | | 
>
> where we go straight down. I'm not sure I've fully grasped it, but it
> feels like that distinction is the source of the off-by-one. I'm not
> sure how to tell the difference here, though. I think it relies on the
> next commit on the left-hand line being the same as the first parent (or
> maybe any parent?).
>
> If I remove the use of parent_in_old_cols entirely, the merge looks
> right, but the "collapsing" one is broken (and t4214 fails).
>
> By the way, a useful trick I stumbled on to look at the coloring across
> many such merges:
>
>   git log --graph --format=%h --color | grep -A2 -e - | less -S
>
> It looks like every octopus in git.git is colored wrong (because they're
> the non-collapsing type).

Thanks for analysing further.  I wonder if new tests added by
Denton's BUG/PATCH series cover both kinds?  It would be good
to make sure that any "fix" corrects known-broken cases while
keeping the working cases still working.

Thanks.

[PATCH v2 0/5] t4214: cleanup and demonstrate graph bug

[Denton Liu]

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

[PATCH v2 1/5] test-lib: let test_merge() perform octopus merges

[Denton Liu]

Currently test_merge() only allows developers to merge in one branch.
However, this restriction is artificial and there is no reason why it
needs to be this way.

Extend test_merge() to allow the specification of multiple branches so
that octopus merges can be performed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/test-lib-functions.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 87bf3a2287..b299ecc326 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -228,9 +228,11 @@ test_commit () {
 # can be a tag pointing to the commit-to-merge.
 
 test_merge () {
+	label="$1" &&
+	shift &&
 	test_tick &&
-	git merge -m "$1" "$2" &&
-	git tag "$1"
+	git merge -m "$label" "$@" &&
+	git tag "$label"
 }
 
 # Efficiently create <nr> commits, each with a unique number (from 1 to <nr>
-- 
2.23.0.565.g1cc52d20df

[PATCH v2 2/5] t4214: use test_merge

[Denton Liu]

In the previous commit, we extended test_merge() so that it could
perform octopus merges. Now that the restriction is lifted, use
test_merge() to perform the octopus merge instead of manually
duplicating test_merge() functionality.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index dab96c89aa..f6e22ec825 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -41,8 +41,7 @@ test_expect_success 'set up merge history' '
 		test_commit $i $i $i tag$i || return $?
 	done &&
 	git checkout 1 -b merge &&
-	test_tick &&
-	git merge -m octopus-merge 1 2 3 4 &&
+	test_merge octopus-merge 1 2 3 4 &&
 	git checkout 1 -b L &&
 	test_commit left
 '
-- 
2.23.0.565.g1cc52d20df

[PATCH v2 3/5] t4214: generate expect in their own test cases

[Denton Liu]

Before, the expect files of the test case were being generated in the
setup method. However, it would make more sense to generate these files
within the test cases that actually use them so that it's obvious to
future readers where the expected values are coming from.

Move the generation of the expect files in their own respective test
cases.

While we're at it, we want to establish a pattern in this test suite
that, firstly, a non-colored test case is given then, immediately after,
the colored version is given.

Switch test cases "log --graph with tricky octopus merge, no color" and
"log --graph with tricky octopus merge with colors" so that the "no
color" version appears first.

This patch is best viewed with `--color-moved`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 42 ++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index f6e22ec825..16776e347c 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -5,6 +5,20 @@ test_description='git log --graph of skewed left octopus merge.'
 . ./test-lib.sh
 
 test_expect_success 'set up merge history' '
+	test_commit initial &&
+	for i in 1 2 3 4 ; do
+		git checkout master -b $i || return $?
+		# Make tag name different from branch name, to avoid
+		# ambiguity error when calling checkout.
+		test_commit $i $i $i tag$i || return $?
+	done &&
+	git checkout 1 -b merge &&
+	test_merge octopus-merge 1 2 3 4 &&
+	git checkout 1 -b L &&
+	test_commit left
+'
+
+test_expect_success 'log --graph with tricky octopus merge, no color' '
 	cat >expect.uncolored <<-\EOF &&
 	* left
 	| *---.   octopus-merge
@@ -19,6 +33,13 @@ test_expect_success 'set up merge history' '
 	|/
 	* initial
 	EOF
+	git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
+	sed "s/ *\$//" actual.raw >actual &&
+	test_cmp expect.uncolored actual
+'
+
+test_expect_success 'log --graph with tricky octopus merge with colors' '
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
 	cat >expect.colors <<-\EOF &&
 	* left
 	<RED>|<RESET> *<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET>   octopus-merge
@@ -33,32 +54,11 @@ test_expect_success 'set up merge history' '
 	<MAGENTA>|<RESET><MAGENTA>/<RESET>
 	* initial
 	EOF
-	test_commit initial &&
-	for i in 1 2 3 4 ; do
-		git checkout master -b $i || return $?
-		# Make tag name different from branch name, to avoid
-		# ambiguity error when calling checkout.
-		test_commit $i $i $i tag$i || return $?
-	done &&
-	git checkout 1 -b merge &&
-	test_merge octopus-merge 1 2 3 4 &&
-	git checkout 1 -b L &&
-	test_commit left
-'
-
-test_expect_success 'log --graph with tricky octopus merge with colors' '
-	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
 	git log --color=always --graph --date-order --pretty=tformat:%s --all >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, no color' '
-	git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
-	sed "s/ *\$//" actual.raw >actual &&
-	test_cmp expect.uncolored actual
-'
-
 # Repeat the previous two tests with "normal" octopus merge (i.e.,
 # without the first parent skewing to the "left" branch column).
 
-- 
2.23.0.565.g1cc52d20df

[PATCH v2 4/5] t4214: explicitly list tags in log

[Denton Liu]

In a future test case, we will be extending the commit graph. As a
result, explicitly list the tags that will generate the graph so that
when future additions are made, the current graph illustrations won't be
affected.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 16776e347c..097151da39 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -33,7 +33,7 @@ test_expect_success 'log --graph with tricky octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
+	git log --color=never --graph --date-order --pretty=tformat:%s left octopus-merge >actual.raw &&
 	sed "s/ *\$//" actual.raw >actual &&
 	test_cmp expect.uncolored actual
 '
@@ -54,7 +54,7 @@ test_expect_success 'log --graph with tricky octopus merge with colors' '
 	<MAGENTA>|<RESET><MAGENTA>/<RESET>
 	* initial
 	EOF
-	git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw &&
+	git log --color=always --graph --date-order --pretty=tformat:%s left octopus-merge >actual.colors.raw &&
 	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
 	test_cmp expect.colors actual.colors
 '
@@ -75,7 +75,7 @@ test_expect_success 'log --graph with normal octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s merge >actual.raw &&
+	git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw &&
 	sed "s/ *\$//" actual.raw >actual &&
 	test_cmp expect.uncolored actual
 '
@@ -94,7 +94,7 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 	* initial
 	EOF
 	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
-	git log --color=always --graph --date-order --pretty=tformat:%s merge >actual.colors.raw &&
+	git log --color=always --graph --date-order --pretty=tformat:%s octopus-merge >actual.colors.raw &&
 	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
 	test_cmp expect.colors actual.colors
 '
-- 
2.23.0.565.g1cc52d20df

[PATCH v2 5/5] t4214: demonstrate octopus graph coloring failure

[Denton Liu]

The graph coloring logic for octopus merges currently has a bug. This
can be seen git.git with 74c7cfa875 (Merge of
http://members.cox.net/junkio/git-jc.git, 2005-05-05), whose second
child is 211232bae6 (Octopus merge of the following five patches.,
2005-05-05).

If one runs

	git log --graph 74c7cfa875

one can see that the octopus merge is colored incorrectly. In
particular, the horizontal dashes are off by one color. Each horizontal
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 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.

While we're at it, include a passing test case as a canary in case an
attempt to fix the bug breaks existing operation.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 282 ++++++++++++++++++++++++++++++++++-
 1 file changed, 281 insertions(+), 1 deletion(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 097151da39..3ae8e51e50 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -14,8 +14,13 @@ test_expect_success 'set up merge history' '
 	done &&
 	git checkout 1 -b merge &&
 	test_merge octopus-merge 1 2 3 4 &&
+	test_commit after-merge &&
 	git checkout 1 -b L &&
-	test_commit left
+	test_commit left &&
+	git checkout 4 -b crossover &&
+	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' '
@@ -98,4 +103,279 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
 	test_cmp expect.colors actual.colors
 '
+
+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
+	| *---.   octopus-merge
+	| |\ \ \
+	|/ / / /
+	| | | * 4
+	| | * | 3
+	| | |/
+	| * | 2
+	| |/
+	* | 1
+	|/
+	* initial
+	EOF
+	git log --color=never --graph --date-order --pretty=tformat:%s left after-merge >actual.raw &&
+	sed "s/ *\$//" actual.raw >actual &&
+	test_cmp expect.uncolored actual
+'
+
+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
+	<RED>|<RESET> * after-merge
+	<RED>|<RESET> *<MAGENTA>-<RESET><MAGENTA>-<RESET><CYAN>-<RESET><CYAN>.<RESET>   octopus-merge
+	<RED>|<RESET> <RED>|<RESET><BLUE>\<RESET> <MAGENTA>\<RESET> <CYAN>\<RESET>
+	<RED>|<RESET><RED>/<RESET> <BLUE>/<RESET> <MAGENTA>/<RESET> <CYAN>/<RESET>
+	<RED>|<RESET> <BLUE>|<RESET> <MAGENTA>|<RESET> * 4
+	<RED>|<RESET> <BLUE>|<RESET> * <CYAN>|<RESET> 3
+	<RED>|<RESET> <BLUE>|<RESET> <CYAN>|<RESET><CYAN>/<RESET>
+	<RED>|<RESET> * <CYAN>|<RESET> 2
+	<RED>|<RESET> <CYAN>|<RESET><CYAN>/<RESET>
+	* <CYAN>|<RESET> 1
+	<CYAN>|<RESET><CYAN>/<RESET>
+	* initial
+	EOF
+	git log --color=always --graph --date-order --pretty=tformat:%s left 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 crossover in octopus merge, no color' '
+	cat >expect.uncolored <<-\EOF &&
+	* after-4
+	| *---.   octopus-merge
+	| |\ \ \
+	| |_|_|/
+	|/| | |
+	* | | | 4
+	| | | * 3
+	| |_|/
+	|/| |
+	| | * 2
+	| |/
+	|/|
+	| * 1
+	|/
+	* initial
+	EOF
+	git log --color=never --graph --date-order --pretty=tformat:%s after-4 octopus-merge >actual.raw &&
+	sed "s/ *\$//" actual.raw >actual &&
+	test_cmp expect.uncolored actual
+'
+
+test_expect_failure 'log --graph with crossover in octopus merge with colors' '
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
+	cat >expect.colors <<-\EOF &&
+	* after-4
+	<RED>|<RESET> *<BLUE>-<RESET><BLUE>-<RESET><RED>-<RESET><RED>.<RESET>   octopus-merge
+	<RED>|<RESET> <GREEN>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> <RED>\<RESET>
+	<RED>|<RESET> <GREEN>|<RESET><RED>_<RESET><YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>/<RESET>
+	<RED>|<RESET><RED>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET>
+	* <GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> 4
+	<MAGENTA>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> * 3
+	<MAGENTA>|<RESET> <GREEN>|<RESET><MAGENTA>_<RESET><YELLOW>|<RESET><MAGENTA>/<RESET>
+	<MAGENTA>|<RESET><MAGENTA>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET>
+	<MAGENTA>|<RESET> <GREEN>|<RESET> * 2
+	<MAGENTA>|<RESET> <GREEN>|<RESET><MAGENTA>/<RESET>
+	<MAGENTA>|<RESET><MAGENTA>/<RESET><GREEN>|<RESET>
+	<MAGENTA>|<RESET> * 1
+	<MAGENTA>|<RESET><MAGENTA>/<RESET>
+	* initial
+	EOF
+	git log --color=always --graph --date-order --pretty=tformat:%s after-4 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 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

Re: [PATCH v2 0/5] t4214: cleanup and demonstrate graph bug

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> Range-diff against v1:
> 1:  e77af8cde5 = 1:  e77af8cde5 test-lib: let test_merge() perform octopus merges

micronit: I would say s/let/allow/ if I were writing this.

> 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

Queued and pushed out.  Thanks.