From 94ba151300ca5e4345a11b00244eb3501bc569f8 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 3 Oct 2019 17:23:13 -0700 Subject: [PATCH 1/5] test-lib: let test_merge() perform octopus merges 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 Signed-off-by: Junio C Hamano --- 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 e0b3f28d3a..6620ef2f34 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 commits, each with a unique number (from 1 to From a7a5590c6ecd75157c24f6dac1624b0e70afeb23 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 3 Oct 2019 17:23:15 -0700 Subject: [PATCH 2/5] t4214: use test_merge 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 Signed-off-by: Junio C Hamano --- 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 ' From 63be8c8dd7f6ddb90613162b0dc05fa11ee60eec Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 3 Oct 2019 17:23:17 -0700 Subject: [PATCH 3/5] t4214: generate expect in their own test cases 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 Signed-off-by: Junio C Hamano --- 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 | *---. octopus-merge @@ -33,32 +54,11 @@ test_expect_success 'set up merge history' ' |/ * 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 && 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). From 25eb905e14eb3dd72e3208735efd2aa0ef90c550 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 3 Oct 2019 17:23:20 -0700 Subject: [PATCH 4/5] t4214: explicitly list tags in log 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 Signed-off-by: Junio C Hamano --- 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' ' |/ * 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 && 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 && test_cmp expect.colors actual.colors ' From 11c21f22ded635bec5e10c833697c55aab442776 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 3 Oct 2019 17:23:22 -0700 Subject: [PATCH 5/5] t4214: demonstrate octopus graph coloring failure 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 Signed-off-by: Junio C Hamano --- 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 && 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 + *---. octopus-merge + |\ \ \ + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * 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 && + 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 + | * after-merge + | *---. octopus-merge + | |\ \ \ + |/ / / / + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s left after-merge >actual.colors.raw && + test_decode_color 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 + | *---. octopus-merge + | |\ \ \ + | |_|_|/ + |/| | | + * | | | 4 + | | | * 3 + | |_|/ + |/| | + | | * 2 + | |/ + |/| + | * 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s after-4 octopus-merge >actual.colors.raw && + test_decode_color 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 + | * after-merge + | *---. octopus-merge + | |\ \ \ + | |_|_|/ + |/| | | + * | | | 4 + | | | * 3 + | |_|/ + |/| | + | | * 2 + | |/ + |/| + | * 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s after-4 after-merge >actual.colors.raw && + test_decode_color 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 + | *---. octopus-merge + | |\ \ \ + | | | | * 4 + | |_|_|/ + |/| | | + | | | * 3 + | |_|/ + |/| | + | | * 2 + | |/ + |/| + | * 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s after-initial octopus-merge >actual.colors.raw && + test_decode_color 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 + | * after-merge + | *---. octopus-merge + | |\ \ \ + | | | | * 4 + | |_|_|/ + |/| | | + | | | * 3 + | |_|/ + |/| | + | | * 2 + | |/ + |/| + | * 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s after-initial after-merge >actual.colors.raw && + test_decode_color actual.colors && + test_cmp expect.colors actual.colors +' + test_done