From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Allan Caffee <allan.caffee@gmail.com>,
Noam Postavsky <npostavs@users.sourceforge.net>,
Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/5] t4214: cleanup and demonstrate graph bug
Date: Thu, 3 Oct 2019 17:23:09 -0700 [thread overview]
Message-ID: <cover.1570148053.git.liu.denton@gmail.com> (raw)
In-Reply-To: <cover.1569407150.git.liu.denton@gmail.com>
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
next prev parent reply other threads:[~2019-10-04 0:23 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-25 10:26 [BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug Denton Liu
2019-09-25 10:27 ` [BUG/PATCH 1/5] test-lib: let test_merge() perform octopus merges Denton Liu
2019-09-25 10:27 ` [BUG/PATCH 2/5] t4214: use test_merge Denton Liu
2019-09-25 10:27 ` [BUG/PATCH 3/5] t4214: generate expect in their own test cases Denton Liu
2019-09-25 10:27 ` [BUG/PATCH 4/5] t4214: explicitly list tags in log Denton Liu
2019-09-25 10:27 ` [BUG/PATCH 5/5] t4214: demonstrate octopus graph coloring failure Denton Liu
2019-09-25 17:09 ` [BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug Denton Liu
2019-09-26 20:23 ` Jeff King
2019-10-03 22:16 ` Junio C Hamano
2019-10-04 0:23 ` Denton Liu [this message]
2019-10-04 0:23 ` [PATCH v2 1/5] test-lib: let test_merge() perform octopus merges Denton Liu
2019-10-04 0:23 ` [PATCH v2 2/5] t4214: use test_merge Denton Liu
2019-10-04 0:23 ` [PATCH v2 3/5] t4214: generate expect in their own test cases Denton Liu
2019-10-04 0:23 ` [PATCH v2 4/5] t4214: explicitly list tags in log Denton Liu
2019-10-04 0:23 ` [PATCH v2 5/5] t4214: demonstrate octopus graph coloring failure Denton Liu
2019-10-04 5:50 ` [PATCH v2 0/5] t4214: cleanup and demonstrate graph bug Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1570148053.git.liu.denton@gmail.com \
--to=liu.denton@gmail.com \
--cc=allan.caffee@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=npostavs@users.sourceforge.net \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.