[GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp

git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed

* [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic
@ 2020-02-16 13:47 Abhishek Kumar
  2020-02-16 13:47 ` [GSoC Patch 2/5] t3430: use lib-log-graph functions Abhishek Kumar
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-16 13:47 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Logic for comparing log graphs is duplicated across test scripts.

This patchset consolidates such logic into lib-log-graph.

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
1. I don't think this patchset requires a cover letter or extended
commit descriptions - Fairly simple, straightforward changes.
2. This patchset closes issue #471 from gitgitgadget.

 t/lib-log-graph.sh | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 t/lib-log-graph.sh

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
new file mode 100644
index 0000000000..999f2600de
--- /dev/null
+++ b/t/lib-log-graph.sh
@@ -0,0 +1,39 @@
+# Helpers shared by the test scripts for comparing log graphs.
+
+sanitize_output() {
+	sed -e 's/ *$//' \
+	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
+	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
+	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
+	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
+	    -e 's/, 0 deletions(-)//' \
+	    -e 's/, 0 insertions(+)//' \
+	    -e 's/ 1 files changed, / 1 file changed, /' \
+	    -e 's/, 1 deletions(-)/, 1 deletion(-)/' \
+	    -e 's/, 1 insertions(+)/, 1 insertion(+)/' \
+	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
+}
+
+# Assume expected graph is in file `expect`
+test_cmp_graph_file() {
+	git log --graph "$@" >output &&
+	sanitize_output >output.trimmed <output &&
+	test_i18ncmp expect output.trimmed
+}
+
+test_cmp_graph() {
+	cat >expect &&
+	test_cmp_graph_file "$@"
+}
+
+# Assume expected graph is in file `expect.colors`
+test_cmp_colored_graph_file() {
+	git log --graph --color=always "$@" >output.colors.raw &&
+	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
+	test_cmp expect.colors output.colors
+}
+
+test_cmp_colored_graph() {
+	cat >expect.colors &&
+	test_cmp_colored_graph_file "$@"
+}
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC Patch 2/5] t3430: use lib-log-graph functions
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
@ 2020-02-16 13:47 ` Abhishek Kumar
  2020-02-19 17:23   ` Junio C Hamano
  2020-02-16 13:47 ` [GSoC Patch 3/5] t4215: " Abhishek Kumar
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-16 13:47 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/t3430-rebase-merges.sh | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index e72ca348ea..74c61fa787 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -20,13 +20,7 @@ Initial setup:
 '
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh
-
-test_cmp_graph () {
-	cat >expect &&
-	git log --graph --boundary --format=%s "$@" >output &&
-	sed "s/ *$//" <output >output.trimmed &&
-	test_cmp expect output.trimmed
-}
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 test_expect_success 'setup' '
 	write_script replace-editor.sh <<-\EOF &&
@@ -84,7 +78,7 @@ test_expect_success 'create completely different structure' '
 	test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
 	test_tick &&
 	git rebase -i -r A master &&
-	test_cmp_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --boundary <<-\EOF
 	*   Merge the topic branch '\''onebranch'\''
 	|\
 	| * D
@@ -201,7 +195,7 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
 	git checkout already-upstream &&
 	test_tick &&
 	git rebase -i -r upstream-with-a2 &&
-	test_cmp_graph upstream-with-a2.. <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --boundary upstream-with-a2.. <<-\EOF
 	*   Merge branch A
 	|\
 	| * A1
@@ -219,7 +213,7 @@ test_expect_success 'do not rebase cousins unless asked for' '
 	test_cmp_rev HEAD $before &&
 	test_tick &&
 	git rebase --rebase-merges=rebase-cousins HEAD^ &&
-	test_cmp_graph HEAD^.. <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --boundary HEAD^.. <<-\EOF
 	*   Merge the topic branch '\''onebranch'\''
 	|\
 	| * D
@@ -311,7 +305,7 @@ test_expect_success 'root commits' '
 	test $(git rev-parse second-root^0) != $(git rev-parse HEAD^) &&
 	test $(git rev-parse second-root:second-root.t) = \
 		$(git rev-parse HEAD^:second-root.t) &&
-	test_cmp_graph HEAD <<-\EOF &&
+	test_cmp_graph --pretty=tformat:%s --boundary HEAD <<-\EOF &&
 	*   Merge the 3rd root
 	|\
 	| * third-root
@@ -347,7 +341,7 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '
 	test_tick &&
 	git rebase -f -r HEAD^ &&
 	test_cmp_rev ! HEAD^2 khnum &&
-	test_cmp_graph HEAD^.. <<-\EOF &&
+	test_cmp_graph --pretty=tformat:%s --boundary HEAD^.. <<-\EOF &&
 	*   Merge branch '\''khnum'\'' into asherah
 	|\
 	| * yama
@@ -355,7 +349,7 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '
 	EOF
 	test_tick &&
 	git rebase --rebase-merges=rebase-cousins HEAD^ &&
-	test_cmp_graph HEAD^.. <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --boundary HEAD^.. <<-\EOF
 	*   Merge branch '\''khnum'\'' into asherah
 	|\
 	| * yama
@@ -402,7 +396,7 @@ test_expect_success 'octopus merges' '
 	git rebase -i --force-rebase -r HEAD^^ &&
 	test "Hank" = "$(git show -s --format=%an HEAD)" &&
 	test "$before" != $(git rev-parse HEAD) &&
-	test_cmp_graph HEAD^^.. <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --boundary HEAD^^.. <<-\EOF
 	*-.   Tüntenfüsch
 	|\ \
 	| | * three
@@ -478,7 +472,7 @@ test_expect_success '--rebase-merges with message matched with onto label' '
 	git checkout -b onto-label E &&
 	git merge -m onto G &&
 	git rebase --rebase-merges --force-rebase E &&
-	test_cmp_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --boundary <<-\EOF
 	*   onto
 	|\
 	| * G
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC Patch 3/5] t4215: use lib-log-graph functions
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
  2020-02-16 13:47 ` [GSoC Patch 2/5] t3430: use lib-log-graph functions Abhishek Kumar
@ 2020-02-16 13:47 ` Abhishek Kumar
  2020-02-19 17:26   ` Junio C Hamano
  2020-02-16 13:47 ` [GSoC Patch 4/5] t4214: " Abhishek Kumar
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-16 13:47 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/t4215-log-skewed-merges.sh | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 1d0d3240ff..bca478cb83 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -3,13 +3,7 @@
 test_description='git log --graph of skewed merges'
 
 . ./test-lib.sh
-
-check_graph () {
-	cat >expect &&
-	git log --graph --pretty=tformat:%s "$@" >actual.raw &&
-	sed "s/ *$//" actual.raw >actual &&
-	test_cmp expect actual
-}
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
 	git checkout --orphan _p &&
@@ -22,7 +16,7 @@ test_expect_success 'log --graph with merge fusing with its left and right neigh
 	git checkout _p && git merge --no-ff _r -m G &&
 	git checkout @^^ && git merge --no-ff _p -m H &&
 
-	check_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s <<-\EOF
 	*   H
 	|\
 	| *   G
@@ -50,7 +44,7 @@ test_expect_success 'log --graph with left-skewed merge' '
 	git checkout 0_p && git merge --no-ff 0_s -m 0_G &&
 	git checkout @^ && git merge --no-ff 0_q 0_r 0_t 0_p -m 0_H &&
 
-	check_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s <<-\EOF
 	*-----.   0_H
 	|\ \ \ \
 	| | | | * 0_G
@@ -84,7 +78,7 @@ test_expect_success 'log --graph with nested left-skewed merge' '
 	git checkout 1_p && git merge --no-ff 1_r -m 1_G &&
 	git checkout @^^ && git merge --no-ff 1_p -m 1_H &&
 
-	check_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s <<-\EOF
 	*   1_H
 	|\
 	| *   1_G
@@ -116,7 +110,7 @@ test_expect_success 'log --graph with nested left-skewed merge following normal
 	git checkout -b 2_s @^^ && git merge --no-ff 2_q -m 2_J &&
 	git checkout 2_p && git merge --no-ff 2_s -m 2_K &&
 
-	check_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s <<-\EOF
 	*   2_K
 	|\
 	| *   2_J
@@ -152,7 +146,7 @@ test_expect_success 'log --graph with nested right-skewed merge following left-s
 	git checkout 3_p && git merge --no-ff 3_r -m 3_H &&
 	git checkout @^^ && git merge --no-ff 3_p -m 3_J &&
 
-	check_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s <<-\EOF
 	*   3_J
 	|\
 	| *   3_H
@@ -183,7 +177,7 @@ test_expect_success 'log --graph with right-skewed merge following a left-skewed
 	git merge --no-ff 4_p -m 4_G &&
 	git checkout @^^ && git merge --no-ff 4_s -m 4_H &&
 
-	check_graph --date-order <<-\EOF
+	test_cmp_graph --pretty=tformat:%s --date-order <<-\EOF
 	*   4_H
 	|\
 	| *   4_G
@@ -219,7 +213,7 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
 	git checkout 5_r &&
 	git merge --no-ff 5_s -m 5_H &&
 
-	check_graph <<-\EOF
+	test_cmp_graph --pretty=tformat:%s <<-\EOF
 	*   5_H
 	|\
 	| *-.   5_G
@@ -258,7 +252,7 @@ test_expect_success 'log --graph with multiple tips' '
 	git checkout 6_1 &&
 	git merge --no-ff 6_2 -m 6_I &&
 
-	check_graph 6_1 6_3 6_5 <<-\EOF
+	test_cmp_graph --pretty=tformat:%s 6_1 6_3 6_5 <<-\EOF
 	*   6_I
 	|\
 	| | *   6_H
@@ -337,7 +331,7 @@ test_expect_success 'log --graph with multiple tips' '
 	git checkout -b M_7 7_1 &&
 	git merge --no-ff 7_2 7_3 -m 7_M4 &&
 
-	check_graph M_1 M_3 M_5 M_7 <<-\EOF
+	test_cmp_graph --pretty=tformat:%s M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC Patch 4/5] t4214: use lib-log-graph functions
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
  2020-02-16 13:47 ` [GSoC Patch 2/5] t3430: use lib-log-graph functions Abhishek Kumar
  2020-02-16 13:47 ` [GSoC Patch 3/5] t4215: " Abhishek Kumar
@ 2020-02-16 13:47 ` Abhishek Kumar
  2020-02-19 17:29   ` Junio C Hamano
  2020-02-16 13:47 ` [GSoC Patch 5/5] t4202: " Abhishek Kumar
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-16 13:47 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/t4214-log-graph-octopus.sh | 86 ++++++++----------------------------
 1 file changed, 19 insertions(+), 67 deletions(-)

diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 40d27db674..e85cf07d2c 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -3,6 +3,7 @@
 test_description='git log --graph of skewed left octopus merge.'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 test_expect_success 'set up merge history' '
 	test_commit initial &&
@@ -24,7 +25,7 @@ test_expect_success 'set up merge history' '
 '
 
 test_expect_success 'log --graph with tricky octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph --pretty=tformat:%s --color=never --date-order left octopus-merge <<-\EOF
 	* left
 	| *-.   octopus-merge
 	|/|\ \
@@ -37,14 +38,11 @@ test_expect_success 'log --graph with tricky octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	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
 '
 
 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 &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order left octopus-merge <<-\EOF
 	* left
 	<RED>|<RESET> *<MAGENTA>-<RESET><MAGENTA>.<RESET>   octopus-merge
 	<RED>|<RESET><RED>/<RESET><YELLOW>|<RESET><BLUE>\<RESET> <MAGENTA>\<RESET>
@@ -57,16 +55,13 @@ 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 left octopus-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
 '
 
 # Repeat the previous two tests with "normal" octopus merge (i.e.,
 # without the first parent skewing to the "left" branch column).
 
 test_expect_success 'log --graph with normal octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph --pretty=tformat:%s --color=never --date-order octopus-merge <<-\EOF
 	*---.   octopus-merge
 	|\ \ \
 	| | | * 4
@@ -78,13 +73,11 @@ test_expect_success 'log --graph with normal octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw &&
-	sed "s/ *\$//" actual.raw >actual &&
-	test_cmp expect.uncolored actual
 '
 
 test_expect_success 'log --graph with normal octopus merge with colors' '
-	cat >expect.colors <<-\EOF &&
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order octopus-merge <<-\EOF
 	*<YELLOW>-<RESET><YELLOW>-<RESET><BLUE>-<RESET><BLUE>.<RESET>   octopus-merge
 	<RED>|<RESET><GREEN>\<RESET> <YELLOW>\<RESET> <BLUE>\<RESET>
 	<RED>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> * 4
@@ -96,14 +89,10 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 	<BLUE>|<RESET><BLUE>/<RESET>
 	* initial
 	EOF
-	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
-	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
 '
 
 test_expect_success 'log --graph with normal octopus merge and child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph --pretty=tformat:%s --color=never --date-order after-merge <<-\EOF
 	* after-merge
 	*---.   octopus-merge
 	|\ \ \
@@ -116,13 +105,11 @@ test_expect_success 'log --graph with normal octopus merge and child, no color'
 	|/
 	* 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_success 'log --graph with normal octopus and child merge with colors' '
-	cat >expect.colors <<-\EOF &&
+	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order after-merge <<-\EOF
 	* after-merge
 	*<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET>   octopus-merge
 	<GREEN>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> <MAGENTA>\<RESET>
@@ -135,14 +122,10 @@ test_expect_success 'log --graph with normal octopus and child merge with colors
 	<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 &&
+	test_cmp_graph --pretty=tformat:%s --color=never --date-order left after-merge <<-\EOF
 	* left
 	| * after-merge
 	| *-.   octopus-merge
@@ -156,14 +139,10 @@ test_expect_success 'log --graph with tricky octopus merge and its child, no col
 	|/
 	* 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_success '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 &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order left after-merge <<-\EOF
 	* left
 	<RED>|<RESET> * after-merge
 	<RED>|<RESET> *<CYAN>-<RESET><CYAN>.<RESET>   octopus-merge
@@ -177,13 +156,10 @@ test_expect_success 'log --graph with tricky octopus merge and its child with co
 	<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 &&
+	test_cmp_graph --pretty=tformat:%s --date-order after-4 octopus-merge <<-\EOF
 	* after-4
 	| *---.   octopus-merge
 	| |\ \ \
@@ -200,14 +176,11 @@ test_expect_success 'log --graph with crossover in octopus merge, no color' '
 	|/
 	* 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_success 'log --graph with crossover in octopus merge with colors' '
 	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
-	cat >expect.colors <<-\EOF &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order after-4 octopus-merge <<-\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>
@@ -224,13 +197,10 @@ test_expect_success 'log --graph with crossover in octopus merge with colors' '
 	<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 &&
+	test_cmp_graph --pretty=tformat:%s --date-order after-4 after-merge <<-\EOF
 	* after-4
 	| * after-merge
 	| *---.   octopus-merge
@@ -248,14 +218,11 @@ test_expect_success 'log --graph with crossover in octopus merge and its child,
 	|/
 	* 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_success '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 &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order after-4 after-merge <<-\EOF
 	* after-4
 	<RED>|<RESET> * after-merge
 	<RED>|<RESET> *<MAGENTA>-<RESET><MAGENTA>-<RESET><RED>-<RESET><RED>.<RESET>   octopus-merge
@@ -273,13 +240,10 @@ test_expect_success 'log --graph with crossover in octopus merge and its child w
 	<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 &&
+	test_cmp_graph --pretty=tformat:%s --color=never --date-order --pretty=tformat:%s after-initial octopus-merge <<-\EOF
 	* after-initial
 	| *---.   octopus-merge
 	| |\ \ \
@@ -296,14 +260,11 @@ test_expect_success 'log --graph with unrelated commit and octopus tip, no color
 	|/
 	* 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 &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order after-initial octopus-merge <<-\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>
@@ -320,13 +281,10 @@ test_expect_success 'log --graph with unrelated commit and octopus tip with colo
 	<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 &&
+	test_cmp_graph --pretty=tformat:%s --color=never --date-order after-initial after-merge <<-\EOF
 	* after-initial
 	| * after-merge
 	| *---.   octopus-merge
@@ -344,14 +302,11 @@ test_expect_success 'log --graph with unrelated commit and octopus child, no col
 	|/
 	* 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_success 'log --graph with unrelated commit and octopus child with colors' '
 	test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
-	cat >expect.colors <<-\EOF &&
+	test_cmp_colored_graph --pretty=tformat:%s --date-order after-initial after-merge <<-\EOF
 	* after-initial
 	<RED>|<RESET> * after-merge
 	<RED>|<RESET> *<MAGENTA>-<RESET><MAGENTA>-<RESET><CYAN>-<RESET><CYAN>.<RESET>   octopus-merge
@@ -369,9 +324,6 @@ test_expect_success 'log --graph with unrelated commit and octopus child with co
 	<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.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC Patch 5/5] t4202: use lib-log-graph functions
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
                   ` (2 preceding siblings ...)
  2020-02-16 13:47 ` [GSoC Patch 4/5] t4214: " Abhishek Kumar
@ 2020-02-16 13:47 ` Abhishek Kumar
  2020-02-19 17:31   ` Junio C Hamano
  2020-02-17  0:05 ` [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Junio C Hamano
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-16 13:47 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/t4202-log.sh | 49 ++++++++++---------------------------------------
 1 file changed, 10 insertions(+), 39 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 192347a3e1..403d88bb33 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -5,6 +5,7 @@ test_description='git log'
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-gpg.sh"
 . "$TEST_DIRECTORY/lib-terminal.sh"
+. "$TEST_DIRECTORY/lib-log-graph.sh"
 
 test_expect_success setup '
 
@@ -452,8 +453,7 @@ cat > expect <<EOF
 EOF
 
 test_expect_success 'simple log --graph' '
-	git log --graph --pretty=tformat:%s >actual &&
-	test_cmp expect actual
+	test_cmp_graph_file --pretty=tformat:%s
 '
 
 cat > expect <<EOF
@@ -467,8 +467,7 @@ cat > expect <<EOF
 EOF
 
 test_expect_success 'simple log --graph --line-prefix="123 "' '
-	git log --graph --line-prefix="123 " --pretty=tformat:%s >actual &&
-	test_cmp expect actual
+	test_cmp_graph_file --line-prefix="123 " --pretty=tformat:%s
 '
 
 test_expect_success 'set up merge history' '
@@ -495,9 +494,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph with merge' '
-	git log --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph_file --date-order --pretty=tformat:%s
 '
 
 cat > expect <<\EOF
@@ -516,9 +513,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph --line-prefix="| | | " with merge' '
-	git log --line-prefix="| | | " --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph_file --line-prefix="| | | " --date-order --pretty=tformat:%s
 '
 
 cat > expect.colors <<\EOF
@@ -538,9 +533,7 @@ EOF
 
 test_expect_success 'log --graph with merge with log.graphColors' '
 	test_config log.graphColors " blue,invalid-color, cyan, red  , " &&
-	git log --color=always --graph --date-order --pretty=tformat:%s |
-		test_decode_color | sed "s/ *\$//" >actual &&
-	test_cmp expect.colors actual
+	test_cmp_colored_graph_file --date-order --pretty=tformat:%s
 '
 
 test_expect_success 'log --raw --graph -m with merge' '
@@ -1213,24 +1206,8 @@ cat >expect <<\EOF
   +one
 EOF
 
-sanitize_output () {
-	sed -e 's/ *$//' \
-	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
-	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
-	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
-	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
-	    -e 's/, 0 deletions(-)//' \
-	    -e 's/, 0 insertions(+)//' \
-	    -e 's/ 1 files changed, / 1 file changed, /' \
-	    -e 's/, 1 deletions(-)/, 1 deletion(-)/' \
-	    -e 's/, 1 insertions(+)/, 1 insertion(+)/' \
-	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
-}
-
 test_expect_success 'log --graph with diff and stats' '
-	git log --no-renames --graph --pretty=short --stat -p >actual &&
-	sanitize_output >actual.sanitized <actual &&
-	test_i18ncmp expect actual.sanitized
+	test_cmp_graph_file --no-renames --graph --pretty=short --stat -p
 '
 
 cat >expect <<\EOF
@@ -1505,9 +1482,7 @@ cat >expect <<\EOF
 EOF
 
 test_expect_success 'log --line-prefix="*** " --graph with diff and stats' '
-	git log --line-prefix="*** " --no-renames --graph --pretty=short --stat -p >actual &&
-	sanitize_output >actual.sanitized <actual &&
-	test_i18ncmp expect actual.sanitized
+	test_cmp_graph_file --line-prefix="*** " --no-renames --graph --pretty=short --stat -p
 '
 
 cat >expect <<-\EOF
@@ -1529,9 +1504,7 @@ cat >expect <<-\EOF
 EOF
 
 test_expect_success 'log --graph with --name-status' '
-	git log --graph --format=%s --name-status tangle..reach >actual &&
-	sanitize_output <actual >actual.sanitized &&
-	test_cmp expect actual.sanitized
+	test_cmp_graph_file --pretty=tformat:%s --name-status tangle..reach
 '
 
 cat >expect <<-\EOF
@@ -1553,9 +1526,7 @@ cat >expect <<-\EOF
 EOF
 
 test_expect_success 'log --graph with --name-only' '
-	git log --graph --format=%s --name-only tangle..reach >actual &&
-	sanitize_output <actual >actual.sanitized &&
-	test_cmp expect actual.sanitized
+	test_cmp_graph_file --pretty=tformat:%s --name-only tangle..reach
 '
 
 test_expect_success 'dotdot is a parent directory' '
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
                   ` (3 preceding siblings ...)
  2020-02-16 13:47 ` [GSoC Patch 5/5] t4202: " Abhishek Kumar
@ 2020-02-17  0:05 ` Junio C Hamano
  2020-02-19 17:32   ` Junio C Hamano
  2020-02-20  9:15 ` [GSoC PATCH v2 0/2] Consolidate " Abhishek Kumar
  2020-02-24 13:38 ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Abhishek Kumar
  6 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2020-02-17  0:05 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, Johannes Schindelin

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> Logic for comparing log graphs is duplicated across test scripts.
> ...
>  t/lib-log-graph.sh | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 t/lib-log-graph.sh

The presentation order of the patches may be less than ideal, in
that it introduces totally unused code in step 1/5 that is hard to
compare with what it will be used to replace with, and it is
impossible to tell if the potential issues readers see in this step
are merely inherited from existing tests or new issues introduced by
this series, before reading the later steps.

> diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
> new file mode 100644
> index 0000000000..999f2600de
> --- /dev/null
> +++ b/t/lib-log-graph.sh
> @@ -0,0 +1,39 @@
> +# Helpers shared by the test scripts for comparing log graphs.
> +
> +sanitize_output() {

One SP around both sides of ().  I suspect that all helper functions
in this patch has this style violation.

As a library-ish function that can be used outside individual test
script, "output" without any clarification is too broad a word to
act as an object of sanitizing.  Is this function to sanitize the
output from "git log"?  Perhaps at the minimum, it should be called
sanitize_log_output then.

> +	sed -e 's/ *$//' \
> +	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
> +	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
> +	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
> +	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \

These are understandable anonymization; so is the last "index" one.

> +	    -e 's/, 0 deletions(-)//' \
> +	    -e 's/, 0 insertions(+)//' \
> +	    -e 's/ 1 files changed, / 1 file changed, /' \
> +	    -e 's/, 1 deletions(-)/, 1 deletion(-)/' \
> +	    -e 's/, 1 insertions(+)/, 1 insertion(+)/' \

These might deserve comments.  IIUC, all of these are historical
accident and no longer necessary.

> +	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
> +}
> +
> +# Assume expected graph is in file `expect`
> +test_cmp_graph_file() {
> +	git log --graph "$@" >output &&
> +	sanitize_output >output.trimmed <output &&

Pay attention to the names.  If you are "sanitizing", then the
result is not "trimmed".  Call it "sanitized".

> +	test_i18ncmp expect output.trimmed
> +}
> +
> +test_cmp_graph() {
> +	cat >expect &&
> +	test_cmp_graph_file "$@"
> +}

I am not sure if this wrapper is useful or obscuring.  Open coding
the caller of this wrapper, i.e.

	cat >expect <<-\EOF &&
	expected pattern
	EOF
	test_cmp_graph_file $args

is not all that cumbersome, and it might make it more transparent to
the readers what is going on.  I'd need to see the callsites in
later steps to decide it is a good idea.

> +# Assume expected graph is in file `expect.colors`
> +test_cmp_colored_graph_file() {
> +	git log --graph --color=always "$@" >output.colors.raw &&
> +	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
> +	test_cmp expect.colors output.colors
> +}
> +
> +test_cmp_colored_graph() {
> +	cat >expect.colors &&
> +	test_cmp_colored_graph_file "$@"
> +}

So unlike test_cmp_graph family, colored counterparts do not
anonymize?  That sounds a bit harder to use, but we cannot really
tell if that is an issue before seeing the callsites in later steps.

Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch 2/5] t3430: use lib-log-graph functions
  2020-02-16 13:47 ` [GSoC Patch 2/5] t3430: use lib-log-graph functions Abhishek Kumar
@ 2020-02-19 17:23   ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-19 17:23 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, Johannes Schindelin

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
> ---
>  t/t3430-rebase-merges.sh | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> index e72ca348ea..74c61fa787 100755
> --- a/t/t3430-rebase-merges.sh
> +++ b/t/t3430-rebase-merges.sh
> @@ -20,13 +20,7 @@ Initial setup:
>  '
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-rebase.sh
> -
> -test_cmp_graph () {
> -	cat >expect &&
> -	git log --graph --boundary --format=%s "$@" >output &&
> -	sed "s/ *$//" <output >output.trimmed &&
> -	test_cmp expect output.trimmed
> -}
> +. "$TEST_DIRECTORY"/lib-log-graph.sh
>  
>  test_expect_success 'setup' '
>  	write_script replace-editor.sh <<-\EOF &&
> @@ -84,7 +78,7 @@ test_expect_success 'create completely different structure' '
>  	test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
>  	test_tick &&
>  	git rebase -i -r A master &&
> -	test_cmp_graph <<-\EOF
> +	test_cmp_graph --pretty=tformat:%s --boundary <<-\EOF

The original used a more readble short-hand "--format=%s"; was there
a strong reason why we wanted to use "--pretty=tformat:%s"?

The same comment applies to all the following hunks.

I actually have to wonder if this is a good change at all.  Surely
you lost one local and specialized test helper and replaced its use
with a more flexible one from the lib-log-graph file, but because
the one from the lib-log-graph is more flexible, you now need to
tell it what options the tests want to give to the "git log"
command, the same thing over and over, which would make it much more
error prone, no?

It would have been more acceptable if we kept test_cmp_graph a local
and specialized test helper defined in this file, but changed its
implementation (i.e. the 4 lines we see above) to call to a more
generic helper function defined in lib-log-graph file, i.e.

	test_cmp_graph () {
		test_cmp_graph_from_lib --boundary --format=%s "$@"
	}

but then the more flexible helper defined in lib-log-graph file
cannot squat on the short-and-sweet name "test_cmp_graph" that is
already used in the test scripts without unnecessary churn.

I dunno.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch 3/5] t4215: use lib-log-graph functions
  2020-02-16 13:47 ` [GSoC Patch 3/5] t4215: " Abhishek Kumar
@ 2020-02-19 17:26   ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-19 17:26 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, Johannes Schindelin

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
> ---
>  t/t4215-log-skewed-merges.sh | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> index 1d0d3240ff..bca478cb83 100755
> --- a/t/t4215-log-skewed-merges.sh
> +++ b/t/t4215-log-skewed-merges.sh
> @@ -3,13 +3,7 @@
>  test_description='git log --graph of skewed merges'
>  
>  . ./test-lib.sh
> -
> -check_graph () {
> -	cat >expect &&
> -	git log --graph --pretty=tformat:%s "$@" >actual.raw &&
> -	sed "s/ *$//" actual.raw >actual &&
> -	test_cmp expect actual
> -}
> +. "$TEST_DIRECTORY"/lib-log-graph.sh
>  
>  test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
>  	git checkout --orphan _p &&
> @@ -22,7 +16,7 @@ test_expect_success 'log --graph with merge fusing with its left and right neigh
>  	git checkout _p && git merge --no-ff _r -m G &&
>  	git checkout @^^ && git merge --no-ff _p -m H &&
>  
> -	check_graph <<-\EOF
> +	test_cmp_graph --pretty=tformat:%s <<-\EOF

Almost exactly the same comment as [2/5] applies here, but luckily
the name of the local helper used here is check_graph, so we can
discard all the hunks after -22,7 above and instead use

	check_graph () {
		test_cmp_graph --format:%s "$@"
	}

to reduce the code churn, I would think.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch 4/5] t4214: use lib-log-graph functions
  2020-02-16 13:47 ` [GSoC Patch 4/5] t4214: " Abhishek Kumar
@ 2020-02-19 17:29   ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-19 17:29 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, Johannes Schindelin

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> @@ -24,7 +25,7 @@ test_expect_success 'set up merge history' '
>  '
>  
>  test_expect_success 'log --graph with tricky octopus merge, no color' '
> -	cat >expect.uncolored <<-\EOF &&
> +	test_cmp_graph --pretty=tformat:%s --color=never --date-order left octopus-merge <<-\EOF
>  	* left
>  	| *-.   octopus-merge
>  	|/|\ \
> @@ -37,14 +38,11 @@ test_expect_success 'log --graph with tricky octopus merge, no color' '
>  	|/
>  	* initial
>  	EOF
> -	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
>  '
>  
>  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 &&
> +	test_cmp_colored_graph --pretty=tformat:%s --date-order left octopus-merge <<-\EOF
>  	* left
>  	<RED>|<RESET> *<MAGENTA>-<RESET><MAGENTA>.<RESET>   octopus-merge
>  	<RED>|<RESET><RED>/<RESET><YELLOW>|<RESET><BLUE>\<RESET> <MAGENTA>\<RESET>
> @@ -57,16 +55,13 @@ 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 left octopus-merge >actual.colors.raw &&
> -	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
> -	test_cmp expect.colors actual.colors
>  '
> ...

Unlike the previous two steps, this does seem to make the script
cleaner and slightly more readable (it is still unreadable but that
is mostly due to the contents of the here-doc text and cannot be
helped ;-).


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch 5/5] t4202: use lib-log-graph functions
  2020-02-16 13:47 ` [GSoC Patch 5/5] t4202: " Abhishek Kumar
@ 2020-02-19 17:31   ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-19 17:31 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, Johannes Schindelin

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
> ---
>  t/t4202-log.sh | 49 ++++++++++---------------------------------------
>  1 file changed, 10 insertions(+), 39 deletions(-)
>
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 192347a3e1..403d88bb33 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -5,6 +5,7 @@ test_description='git log'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY/lib-gpg.sh"
>  . "$TEST_DIRECTORY/lib-terminal.sh"
> +. "$TEST_DIRECTORY/lib-log-graph.sh"
>  
>  test_expect_success setup '
>  
> @@ -452,8 +453,7 @@ cat > expect <<EOF
>  EOF
>  
>  test_expect_success 'simple log --graph' '
> -	git log --graph --pretty=tformat:%s >actual &&
> -	test_cmp expect actual
> +	test_cmp_graph_file --pretty=tformat:%s
>  '
>  
>  cat > expect <<EOF
> @@ -467,8 +467,7 @@ cat > expect <<EOF
>  EOF
>  
>  test_expect_success 'simple log --graph --line-prefix="123 "' '
> -	git log --graph --line-prefix="123 " --pretty=tformat:%s >actual &&
> -	test_cmp expect actual
> +	test_cmp_graph_file --line-prefix="123 " --pretty=tformat:%s
>  '

Like [4/5], and unlike [2/5] and [3/5], this step also does seem
like an improvement.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic
  2020-02-17  0:05 ` [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Junio C Hamano
@ 2020-02-19 17:32   ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-19 17:32 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, Johannes Schindelin

Here is what I said in the message I am responding to in the patch
form.

 t/lib-log-graph.sh | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
index 999f2600de..bc70f01e84 100644
--- a/t/lib-log-graph.sh
+++ b/t/lib-log-graph.sh
@@ -1,6 +1,12 @@
 # Helpers shared by the test scripts for comparing log graphs.
 
-sanitize_output() {
+sanitize_output () {
+	# Versions of Git that predate 7f814632 ("Use correct grammar
+	# in diffstat summary line", 2012-02-01) did not correctly use
+	# singular when one path was involved, and a handful of rules
+	# were added to work with both older and newer versions of Git
+	# back then.  These are probably not relevant anymore, and
+	# we'd want to lose them someday...
 	sed -e 's/ *$//' \
 	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
 	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
@@ -15,25 +21,25 @@ sanitize_output() {
 }
 
 # Assume expected graph is in file `expect`
-test_cmp_graph_file() {
+test_cmp_graph_file () {
 	git log --graph "$@" >output &&
-	sanitize_output >output.trimmed <output &&
-	test_i18ncmp expect output.trimmed
+	sanitize_output >output.sanitized <output &&
+	test_i18ncmp expect output.sanitized
 }
 
-test_cmp_graph() {
+test_cmp_graph () {
 	cat >expect &&
 	test_cmp_graph_file "$@"
 }
 
 # Assume expected graph is in file `expect.colors`
-test_cmp_colored_graph_file() {
+test_cmp_colored_graph_file () {
 	git log --graph --color=always "$@" >output.colors.raw &&
 	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
 	test_cmp expect.colors output.colors
 }
 
-test_cmp_colored_graph() {
+test_cmp_colored_graph () {
 	cat >expect.colors &&
 	test_cmp_colored_graph_file "$@"
 }
-- 
2.25.1-440-g39558b81cc


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC PATCH v2 0/2] Consolidate test_cmp_graph logic
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
                   ` (4 preceding siblings ...)
  2020-02-17  0:05 ` [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Junio C Hamano
@ 2020-02-20  9:15 ` Abhishek Kumar
  2020-02-20  9:15   ` [GSoC PATCH v2 1/2] lib-log-graph: consolidate " Abhishek Kumar
  2020-02-20  9:15   ` [PATCH v2 2/2] lib-log-graph: consolidate colored graph cmp logic Abhishek Kumar
  2020-02-24 13:38 ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Abhishek Kumar
  6 siblings, 2 replies; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-20  9:15 UTC (permalink / raw)
  To: git

Changes in v2:
- Arrange patches on functions changed, rather than files changed.
- Use specialized helper functions calling library functions.
- Fix SP and other style issues.

I would like to thank Junio for his suggestions - Really cut down on the
needless changes.

Closes gitgitgadget issue #471.

Abhishek Kumar (2):
  lib-log-graph: consolidate test_cmp_graph logic
  lib-log-graph: consolidate colored graph cmp logic

 t/lib-log-graph.sh           | 28 ++++++++++++
 t/t3430-rebase-merges.sh     |  5 +--
 t/t4202-log.sh               | 57 +++++++------------------
 t/t4214-log-graph-octopus.sh | 82 ++++++++++++------------------------
 t/t4215-log-skewed-merges.sh |  9 ++--
 5 files changed, 74 insertions(+), 107 deletions(-)
 create mode 100755 t/lib-log-graph.sh

-- 
2.25.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [GSoC PATCH v2 1/2] lib-log-graph: consolidate test_cmp_graph logic
  2020-02-20  9:15 ` [GSoC PATCH v2 0/2] Consolidate " Abhishek Kumar
@ 2020-02-20  9:15   ` Abhishek Kumar
  2020-02-20 17:49     ` Junio C Hamano
  2020-02-20  9:15   ` [PATCH v2 2/2] lib-log-graph: consolidate colored graph cmp logic Abhishek Kumar
  1 sibling, 1 reply; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-20  9:15 UTC (permalink / raw)
  To: git

Logic for comparing log graphs is duplicated across test scripts.

This patch consolidates such logic into lib-log-graph.

Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/lib-log-graph.sh           | 22 +++++++++++++++
 t/t3430-rebase-merges.sh     |  5 ++--
 t/t4202-log.sh               | 53 ++++++++++--------------------------
 t/t4214-log-graph-octopus.sh | 46 ++++++++++---------------------
 t/t4215-log-skewed-merges.sh |  5 ++--
 5 files changed, 54 insertions(+), 77 deletions(-)
 create mode 100755 t/lib-log-graph.sh

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
new file mode 100755
index 0000000000..97cde44dc7
--- /dev/null
+++ b/t/lib-log-graph.sh
@@ -0,0 +1,22 @@
+# Helps shared by the test scripts for comparing log graphs.
+
+sanitize_log_output () {
+	sed -e 's/ *$//' \
+	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
+	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
+	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
+	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
+	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
+}
+
+lib_test_cmp_graph () {
+	git log --graph "$@" >output &&
+	sed 's/ *$//' >output.sanitized < output &&
+	test_i18ncmp expect output.sanitized
+}
+
+lib_test_cmp_short_graph () {
+	git log --graph --pretty=short "$@" >output &&
+	sanitize_log_output >output.sanitized < output &&
+	test_i18ncmp expect output.sanitized
+}
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index e72ca348ea..a1bc3e2001 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -20,12 +20,11 @@ Initial setup:
 '
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 test_cmp_graph () {
 	cat >expect &&
-	git log --graph --boundary --format=%s "$@" >output &&
-	sed "s/ *$//" <output >output.trimmed &&
-	test_cmp expect output.trimmed
+	lib_test_cmp_graph --boundary --format=%s "$@"
 }
 
 test_expect_success 'setup' '
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 192347a3e1..e025a9cfc2 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -5,6 +5,11 @@ test_description='git log'
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-gpg.sh"
 . "$TEST_DIRECTORY/lib-terminal.sh"
+. "$TEST_DIRECTORY/lib-log-graph.sh"
+
+test_cmp_graph () {
+	lib_test_cmp_graph --format=%s "$@"
+}
 
 test_expect_success setup '
 
@@ -452,8 +457,7 @@ cat > expect <<EOF
 EOF
 
 test_expect_success 'simple log --graph' '
-	git log --graph --pretty=tformat:%s >actual &&
-	test_cmp expect actual
+	test_cmp_graph
 '
 
 cat > expect <<EOF
@@ -467,8 +471,7 @@ cat > expect <<EOF
 EOF
 
 test_expect_success 'simple log --graph --line-prefix="123 "' '
-	git log --graph --line-prefix="123 " --pretty=tformat:%s >actual &&
-	test_cmp expect actual
+	test_cmp_graph --line-prefix="123 "
 '
 
 test_expect_success 'set up merge history' '
@@ -495,9 +498,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph with merge' '
-	git log --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph --date-order
 '
 
 cat > expect <<\EOF
@@ -516,9 +517,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph --line-prefix="| | | " with merge' '
-	git log --line-prefix="| | | " --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph --line-prefix="| | | " --date-order
 '
 
 cat > expect.colors <<\EOF
@@ -676,9 +675,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph with merge' '
-	git log --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph --date-order
 '
 
 test_expect_success 'log.decorate configuration' '
@@ -1213,24 +1210,8 @@ cat >expect <<\EOF
   +one
 EOF
 
-sanitize_output () {
-	sed -e 's/ *$//' \
-	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
-	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
-	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
-	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
-	    -e 's/, 0 deletions(-)//' \
-	    -e 's/, 0 insertions(+)//' \
-	    -e 's/ 1 files changed, / 1 file changed, /' \
-	    -e 's/, 1 deletions(-)/, 1 deletion(-)/' \
-	    -e 's/, 1 insertions(+)/, 1 insertion(+)/' \
-	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
-}
-
 test_expect_success 'log --graph with diff and stats' '
-	git log --no-renames --graph --pretty=short --stat -p >actual &&
-	sanitize_output >actual.sanitized <actual &&
-	test_i18ncmp expect actual.sanitized
+	lib_test_cmp_short_graph --no-renames --stat -p
 '
 
 cat >expect <<\EOF
@@ -1505,9 +1486,7 @@ cat >expect <<\EOF
 EOF
 
 test_expect_success 'log --line-prefix="*** " --graph with diff and stats' '
-	git log --line-prefix="*** " --no-renames --graph --pretty=short --stat -p >actual &&
-	sanitize_output >actual.sanitized <actual &&
-	test_i18ncmp expect actual.sanitized
+	lib_test_cmp_short_graph --line-prefix="*** " --no-renames --stat -p
 '
 
 cat >expect <<-\EOF
@@ -1529,9 +1508,7 @@ cat >expect <<-\EOF
 EOF
 
 test_expect_success 'log --graph with --name-status' '
-	git log --graph --format=%s --name-status tangle..reach >actual &&
-	sanitize_output <actual >actual.sanitized &&
-	test_cmp expect actual.sanitized
+	test_cmp_graph --name-status tangle..reach
 '
 
 cat >expect <<-\EOF
@@ -1553,9 +1530,7 @@ cat >expect <<-\EOF
 EOF
 
 test_expect_success 'log --graph with --name-only' '
-	git log --graph --format=%s --name-only tangle..reach >actual &&
-	sanitize_output <actual >actual.sanitized &&
-	test_cmp expect actual.sanitized
+	test_cmp_graph --name-only tangle..reach
 '
 
 test_expect_success 'dotdot is a parent directory' '
diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 40d27db674..dedb72ace6 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -3,6 +3,12 @@
 test_description='git log --graph of skewed left octopus merge.'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
+
+test_cmp_graph () {
+	cat >expect &&
+	lib_test_cmp_graph --color=never --date-order --format=%s "$@"
+}
 
 test_expect_success 'set up merge history' '
 	test_commit initial &&
@@ -24,7 +30,7 @@ test_expect_success 'set up merge history' '
 '
 
 test_expect_success 'log --graph with tricky octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph left octopus-merge <<-\EOF
 	* left
 	| *-.   octopus-merge
 	|/|\ \
@@ -37,9 +43,6 @@ test_expect_success 'log --graph with tricky octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	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
 '
 
 test_expect_success 'log --graph with tricky octopus merge with colors' '
@@ -66,7 +69,7 @@ test_expect_success 'log --graph with tricky octopus merge with colors' '
 # without the first parent skewing to the "left" branch column).
 
 test_expect_success 'log --graph with normal octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph octopus-merge <<-\EOF
 	*---.   octopus-merge
 	|\ \ \
 	| | | * 4
@@ -78,9 +81,6 @@ test_expect_success 'log --graph with normal octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw &&
-	sed "s/ *\$//" actual.raw >actual &&
-	test_cmp expect.uncolored actual
 '
 
 test_expect_success 'log --graph with normal octopus merge with colors' '
@@ -103,7 +103,7 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 '
 
 test_expect_success 'log --graph with normal octopus merge and child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-merge <<-\EOF
 	* after-merge
 	*---.   octopus-merge
 	|\ \ \
@@ -116,9 +116,6 @@ test_expect_success 'log --graph with normal octopus merge and child, no color'
 	|/
 	* 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_success 'log --graph with normal octopus and child merge with colors' '
@@ -142,7 +139,7 @@ test_expect_success 'log --graph with normal octopus and child merge with colors
 '
 
 test_expect_success 'log --graph with tricky octopus merge and its child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph left after-merge <<-\EOF
 	* left
 	| * after-merge
 	| *-.   octopus-merge
@@ -156,9 +153,6 @@ test_expect_success 'log --graph with tricky octopus merge and its child, no col
 	|/
 	* 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_success 'log --graph with tricky octopus merge and its child with colors' '
@@ -183,7 +177,7 @@ test_expect_success 'log --graph with tricky octopus merge and its child with co
 '
 
 test_expect_success 'log --graph with crossover in octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-4 octopus-merge <<-\EOF
 	* after-4
 	| *---.   octopus-merge
 	| |\ \ \
@@ -200,9 +194,6 @@ test_expect_success 'log --graph with crossover in octopus merge, no color' '
 	|/
 	* 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_success 'log --graph with crossover in octopus merge with colors' '
@@ -230,7 +221,7 @@ test_expect_success 'log --graph with crossover in octopus merge with colors' '
 '
 
 test_expect_success 'log --graph with crossover in octopus merge and its child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-4 after-merge <<-\EOF
 	* after-4
 	| * after-merge
 	| *---.   octopus-merge
@@ -248,9 +239,6 @@ test_expect_success 'log --graph with crossover in octopus merge and its child,
 	|/
 	* 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_success 'log --graph with crossover in octopus merge and its child with colors' '
@@ -279,7 +267,7 @@ test_expect_success 'log --graph with crossover in octopus merge and its child w
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus tip, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-initial octopus-merge <<-\EOF
 	* after-initial
 	| *---.   octopus-merge
 	| |\ \ \
@@ -296,9 +284,6 @@ test_expect_success 'log --graph with unrelated commit and octopus tip, no color
 	|/
 	* 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' '
@@ -326,7 +311,7 @@ test_expect_success 'log --graph with unrelated commit and octopus tip with colo
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-initial after-merge <<-\EOF
 	* after-initial
 	| * after-merge
 	| *---.   octopus-merge
@@ -344,9 +329,6 @@ test_expect_success 'log --graph with unrelated commit and octopus child, no col
 	|/
 	* 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_success 'log --graph with unrelated commit and octopus child with colors' '
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 1d0d3240ff..e1e94176da 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -3,12 +3,11 @@
 test_description='git log --graph of skewed merges'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 check_graph () {
 	cat >expect &&
-	git log --graph --pretty=tformat:%s "$@" >actual.raw &&
-	sed "s/ *$//" actual.raw >actual &&
-	test_cmp expect actual
+	lib_test_cmp_graph --format=%s "$@"
 }
 
 test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] lib-log-graph: consolidate colored graph cmp logic
  2020-02-20  9:15 ` [GSoC PATCH v2 0/2] Consolidate " Abhishek Kumar
  2020-02-20  9:15   ` [GSoC PATCH v2 1/2] lib-log-graph: consolidate " Abhishek Kumar
@ 2020-02-20  9:15   ` Abhishek Kumar
  1 sibling, 0 replies; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-20  9:15 UTC (permalink / raw)
  To: git

Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/lib-log-graph.sh           |  6 ++++++
 t/t4202-log.sh               |  4 +---
 t/t4214-log-graph-octopus.sh | 36 ++++++++++++------------------------
 t/t4215-log-skewed-merges.sh |  4 +---
 4 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
index 97cde44dc7..6185a648ed 100755
--- a/t/lib-log-graph.sh
+++ b/t/lib-log-graph.sh
@@ -20,3 +20,9 @@ lib_test_cmp_short_graph () {
 	sanitize_log_output >output.sanitized < output &&
 	test_i18ncmp expect output.sanitized
 }
+
+lib_test_cmp_colored_graph () {
+	git log --graph --color=always "$@" >output.colors.raw &&
+	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
+	test_cmp expect.colors output.colors
+}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e025a9cfc2..4694b6d0ce 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -537,9 +537,7 @@ EOF
 
 test_expect_success 'log --graph with merge with log.graphColors' '
 	test_config log.graphColors " blue,invalid-color, cyan, red  , " &&
-	git log --color=always --graph --date-order --pretty=tformat:%s |
-		test_decode_color | sed "s/ *\$//" >actual &&
-	test_cmp expect.colors actual
+	lib_test_cmp_colored_graph --date-order --format=%s
 '
 
 test_expect_success 'log --raw --graph -m with merge' '
diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index dedb72ace6..a080325098 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -10,6 +10,10 @@ test_cmp_graph () {
 	lib_test_cmp_graph --color=never --date-order --format=%s "$@"
 }
 
+test_cmp_colored_graph () {
+	lib_test_cmp_colored_graph --date-order --format=%s "$@"
+}
+
 test_expect_success 'set up merge history' '
 	test_commit initial &&
 	for i in 1 2 3 4 ; do
@@ -60,9 +64,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 left octopus-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	test_cmp_colored_graph left octopus-merge
 '
 
 # Repeat the previous two tests with "normal" octopus merge (i.e.,
@@ -97,9 +99,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 octopus-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	test_cmp_colored_graph octopus-merge
 '
 
 test_expect_success 'log --graph with normal octopus merge and child, no color' '
@@ -133,9 +133,7 @@ test_expect_success 'log --graph with normal octopus and child 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 after-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	test_cmp_colored_graph after-merge
 '
 
 test_expect_success 'log --graph with tricky octopus merge and its child, no color' '
@@ -171,9 +169,7 @@ test_expect_success 'log --graph with tricky octopus merge and its child with co
 	<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_cmp_colored_graph left after-merge
 '
 
 test_expect_success 'log --graph with crossover in octopus merge, no color' '
@@ -215,9 +211,7 @@ test_expect_success 'log --graph with crossover in octopus merge with colors' '
 	<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_cmp_colored_graph after-4 octopus-merge
 '
 
 test_expect_success 'log --graph with crossover in octopus merge and its child, no color' '
@@ -261,9 +255,7 @@ test_expect_success 'log --graph with crossover in octopus merge and its child w
 	<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_cmp_colored_graph after-4 after-merge
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus tip, no color' '
@@ -305,9 +297,7 @@ test_expect_success 'log --graph with unrelated commit and octopus tip with colo
 	<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_cmp_colored_graph after-initial octopus-merge
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus child, no color' '
@@ -351,9 +341,7 @@ test_expect_success 'log --graph with unrelated commit and octopus child with co
 	<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_cmp_colored_graph after-initial after-merge
 '
 
 test_done
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index e1e94176da..28d0779a8c 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -305,9 +305,7 @@ test_expect_success 'log --graph with multiple tips and colors' '
 	<BLUE>|<RESET><BLUE>/<RESET>
 	* 6_A
 	EOF
-	git log --color=always --graph --date-order --pretty=tformat:%s 6_1 6_3 6_5 >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	lib_test_cmp_colored_graph --date-order --pretty=tformat:%s 6_1 6_3 6_5
 '
 
 test_expect_success 'log --graph with multiple tips' '
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [GSoC PATCH v2 1/2] lib-log-graph: consolidate test_cmp_graph logic
  2020-02-20  9:15   ` [GSoC PATCH v2 1/2] lib-log-graph: consolidate " Abhishek Kumar
@ 2020-02-20 17:49     ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-20 17:49 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> Logic for comparing log graphs is duplicated across test scripts.
>
> This patch consolidates such logic into lib-log-graph.

The proposed log message is a bit thin.  It does a bit more than
"conslidates", doesn't it?

> Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
> ---
>  t/lib-log-graph.sh           | 22 +++++++++++++++
>  t/t3430-rebase-merges.sh     |  5 ++--
>  t/t4202-log.sh               | 53 ++++++++++--------------------------
>  t/t4214-log-graph-octopus.sh | 46 ++++++++++---------------------
>  t/t4215-log-skewed-merges.sh |  5 ++--
>  5 files changed, 54 insertions(+), 77 deletions(-)
>  create mode 100755 t/lib-log-graph.sh
>
> diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
> new file mode 100755
> index 0000000000..97cde44dc7
> --- /dev/null
> +++ b/t/lib-log-graph.sh
> @@ -0,0 +1,22 @@
> +# Helps shared by the test scripts for comparing log graphs.
> +
> +sanitize_log_output () {
> +	sed -e 's/ *$//' \
> +	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
> +	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
> +	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
> +	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
> +	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
> +}

For example, discarding the singular/plural compat stuff from the
t4202's version is OK, but we should record that is what we did in
the log message to allow readers to notice.

> +lib_test_cmp_graph () {

Nicely named.

> +	git log --graph "$@" >output &&
> +	sed 's/ *$//' >output.sanitized < output &&

Lose SP not just on the output redirection but also on the input
redirection.  I.e.

	sed 's/ *$//' >output.sanitized <output &&

Or you can lose the input redirection altogether as "sed" knows to
treat remaining command line arguments as names of its input files,
i.e.

	sed 's/ *$//' output >output.sanitized &&

People may find the latter easier to read.  I personally do not have
strong preference either way as long as it is consistent.

> +	test_i18ncmp expect output.sanitized
> +}
> +
> +lib_test_cmp_short_graph () {
> +	git log --graph --pretty=short "$@" >output &&
> +	sanitize_log_output >output.sanitized < output &&

	sanitize_log_output <output >output.sanitized &&

(I won't repeat)


>  check_graph () {
>  	cat >expect &&
> -	git log --graph --pretty=tformat:%s "$@" >actual.raw &&
> -	sed "s/ *$//" actual.raw >actual &&
> -	test_cmp expect actual
> +	lib_test_cmp_graph --format=%s "$@"
>  }

Hmm, is this correct?  The input goes to expect but the new helper
you wrote, lib_test_cmp_graph, won't see it.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic
  2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
                   ` (5 preceding siblings ...)
  2020-02-20  9:15 ` [GSoC PATCH v2 0/2] Consolidate " Abhishek Kumar
@ 2020-02-24 13:38 ` Abhishek Kumar
  2020-02-24 13:38   ` [GSoC Patch v3 2/2] lib-log-graph: consolidate colored graph cmp logic Abhishek Kumar
  2020-02-24 21:17   ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Junio C Hamano
  6 siblings, 2 replies; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-24 13:38 UTC (permalink / raw)
  To: abhishekkumar8222; +Cc: git, johannes.schindelin

Log graph comparision logic is duplicated many times in:
- t3430-rebase-merges.sh
- t4202-log.sh
- t4214-log-graph-octopus.sh
- t4215-log-skewed-merges.sh

This patch consolidates comparision and sanitization logic in
lib-log-graph.

Replaces duplicated code with local and lib helpers - making test
scripts cleaner and more readable.

Closes gitgitgadget issue #471

Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
Changes in v3:
- Elaborate on first commit message.
- Fix SP violations in lib-log-graph.

Changes in v2:
- Arrange patches by functions changed instead of files changed.
- Rename library functions by prepending 'lib_'.
- Drop outdated sanitization calls to sed.
- Create specialized helpers to reduce code noise.
- Fix style violations.

 t/lib-log-graph.sh           | 22 +++++++++++++++
 t/t3430-rebase-merges.sh     |  5 ++--
 t/t4202-log.sh               | 53 ++++++++++--------------------------
 t/t4214-log-graph-octopus.sh | 46 ++++++++++---------------------
 t/t4215-log-skewed-merges.sh |  5 ++--
 5 files changed, 54 insertions(+), 77 deletions(-)
 create mode 100755 t/lib-log-graph.sh

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
new file mode 100755
index 0000000000..f9c6526eff
--- /dev/null
+++ b/t/lib-log-graph.sh
@@ -0,0 +1,22 @@
+# Helps shared by the test scripts for comparing log graphs.
+
+sanitize_log_output () {
+	sed -e 's/ *$//' \
+	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
+	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
+	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
+	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
+	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
+}
+
+lib_test_cmp_graph () {
+	git log --graph "$@" >output &&
+	sed 's/ *$//' >output.sanitized <output &&
+	test_i18ncmp expect output.sanitized
+}
+
+lib_test_cmp_short_graph () {
+	git log --graph --pretty=short "$@" >output &&
+	sanitize_log_output >output.sanitized <output &&
+	test_i18ncmp expect output.sanitized
+}
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index e72ca348ea..a1bc3e2001 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -20,12 +20,11 @@ Initial setup:
 '
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 test_cmp_graph () {
 	cat >expect &&
-	git log --graph --boundary --format=%s "$@" >output &&
-	sed "s/ *$//" <output >output.trimmed &&
-	test_cmp expect output.trimmed
+	lib_test_cmp_graph --boundary --format=%s "$@"
 }
 
 test_expect_success 'setup' '
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 192347a3e1..e025a9cfc2 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -5,6 +5,11 @@ test_description='git log'
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-gpg.sh"
 . "$TEST_DIRECTORY/lib-terminal.sh"
+. "$TEST_DIRECTORY/lib-log-graph.sh"
+
+test_cmp_graph () {
+	lib_test_cmp_graph --format=%s "$@"
+}
 
 test_expect_success setup '
 
@@ -452,8 +457,7 @@ cat > expect <<EOF
 EOF
 
 test_expect_success 'simple log --graph' '
-	git log --graph --pretty=tformat:%s >actual &&
-	test_cmp expect actual
+	test_cmp_graph
 '
 
 cat > expect <<EOF
@@ -467,8 +471,7 @@ cat > expect <<EOF
 EOF
 
 test_expect_success 'simple log --graph --line-prefix="123 "' '
-	git log --graph --line-prefix="123 " --pretty=tformat:%s >actual &&
-	test_cmp expect actual
+	test_cmp_graph --line-prefix="123 "
 '
 
 test_expect_success 'set up merge history' '
@@ -495,9 +498,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph with merge' '
-	git log --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph --date-order
 '
 
 cat > expect <<\EOF
@@ -516,9 +517,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph --line-prefix="| | | " with merge' '
-	git log --line-prefix="| | | " --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph --line-prefix="| | | " --date-order
 '
 
 cat > expect.colors <<\EOF
@@ -676,9 +675,7 @@ cat > expect <<\EOF
 EOF
 
 test_expect_success 'log --graph with merge' '
-	git log --graph --date-order --pretty=tformat:%s |
-		sed "s/ *\$//" >actual &&
-	test_cmp expect actual
+	test_cmp_graph --date-order
 '
 
 test_expect_success 'log.decorate configuration' '
@@ -1213,24 +1210,8 @@ cat >expect <<\EOF
   +one
 EOF
 
-sanitize_output () {
-	sed -e 's/ *$//' \
-	    -e 's/commit [0-9a-f]*$/commit COMMIT_OBJECT_NAME/' \
-	    -e 's/Merge: [ 0-9a-f]*$/Merge: MERGE_PARENTS/' \
-	    -e 's/Merge tag.*/Merge HEADS DESCRIPTION/' \
-	    -e 's/Merge commit.*/Merge HEADS DESCRIPTION/' \
-	    -e 's/, 0 deletions(-)//' \
-	    -e 's/, 0 insertions(+)//' \
-	    -e 's/ 1 files changed, / 1 file changed, /' \
-	    -e 's/, 1 deletions(-)/, 1 deletion(-)/' \
-	    -e 's/, 1 insertions(+)/, 1 insertion(+)/' \
-	    -e 's/index [0-9a-f]*\.\.[0-9a-f]*/index BEFORE..AFTER/'
-}
-
 test_expect_success 'log --graph with diff and stats' '
-	git log --no-renames --graph --pretty=short --stat -p >actual &&
-	sanitize_output >actual.sanitized <actual &&
-	test_i18ncmp expect actual.sanitized
+	lib_test_cmp_short_graph --no-renames --stat -p
 '
 
 cat >expect <<\EOF
@@ -1505,9 +1486,7 @@ cat >expect <<\EOF
 EOF
 
 test_expect_success 'log --line-prefix="*** " --graph with diff and stats' '
-	git log --line-prefix="*** " --no-renames --graph --pretty=short --stat -p >actual &&
-	sanitize_output >actual.sanitized <actual &&
-	test_i18ncmp expect actual.sanitized
+	lib_test_cmp_short_graph --line-prefix="*** " --no-renames --stat -p
 '
 
 cat >expect <<-\EOF
@@ -1529,9 +1508,7 @@ cat >expect <<-\EOF
 EOF
 
 test_expect_success 'log --graph with --name-status' '
-	git log --graph --format=%s --name-status tangle..reach >actual &&
-	sanitize_output <actual >actual.sanitized &&
-	test_cmp expect actual.sanitized
+	test_cmp_graph --name-status tangle..reach
 '
 
 cat >expect <<-\EOF
@@ -1553,9 +1530,7 @@ cat >expect <<-\EOF
 EOF
 
 test_expect_success 'log --graph with --name-only' '
-	git log --graph --format=%s --name-only tangle..reach >actual &&
-	sanitize_output <actual >actual.sanitized &&
-	test_cmp expect actual.sanitized
+	test_cmp_graph --name-only tangle..reach
 '
 
 test_expect_success 'dotdot is a parent directory' '
diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index 40d27db674..dedb72ace6 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -3,6 +3,12 @@
 test_description='git log --graph of skewed left octopus merge.'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
+
+test_cmp_graph () {
+	cat >expect &&
+	lib_test_cmp_graph --color=never --date-order --format=%s "$@"
+}
 
 test_expect_success 'set up merge history' '
 	test_commit initial &&
@@ -24,7 +30,7 @@ test_expect_success 'set up merge history' '
 '
 
 test_expect_success 'log --graph with tricky octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph left octopus-merge <<-\EOF
 	* left
 	| *-.   octopus-merge
 	|/|\ \
@@ -37,9 +43,6 @@ test_expect_success 'log --graph with tricky octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	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
 '
 
 test_expect_success 'log --graph with tricky octopus merge with colors' '
@@ -66,7 +69,7 @@ test_expect_success 'log --graph with tricky octopus merge with colors' '
 # without the first parent skewing to the "left" branch column).
 
 test_expect_success 'log --graph with normal octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph octopus-merge <<-\EOF
 	*---.   octopus-merge
 	|\ \ \
 	| | | * 4
@@ -78,9 +81,6 @@ test_expect_success 'log --graph with normal octopus merge, no color' '
 	|/
 	* initial
 	EOF
-	git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw &&
-	sed "s/ *\$//" actual.raw >actual &&
-	test_cmp expect.uncolored actual
 '
 
 test_expect_success 'log --graph with normal octopus merge with colors' '
@@ -103,7 +103,7 @@ test_expect_success 'log --graph with normal octopus merge with colors' '
 '
 
 test_expect_success 'log --graph with normal octopus merge and child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-merge <<-\EOF
 	* after-merge
 	*---.   octopus-merge
 	|\ \ \
@@ -116,9 +116,6 @@ test_expect_success 'log --graph with normal octopus merge and child, no color'
 	|/
 	* 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_success 'log --graph with normal octopus and child merge with colors' '
@@ -142,7 +139,7 @@ test_expect_success 'log --graph with normal octopus and child merge with colors
 '
 
 test_expect_success 'log --graph with tricky octopus merge and its child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph left after-merge <<-\EOF
 	* left
 	| * after-merge
 	| *-.   octopus-merge
@@ -156,9 +153,6 @@ test_expect_success 'log --graph with tricky octopus merge and its child, no col
 	|/
 	* 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_success 'log --graph with tricky octopus merge and its child with colors' '
@@ -183,7 +177,7 @@ test_expect_success 'log --graph with tricky octopus merge and its child with co
 '
 
 test_expect_success 'log --graph with crossover in octopus merge, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-4 octopus-merge <<-\EOF
 	* after-4
 	| *---.   octopus-merge
 	| |\ \ \
@@ -200,9 +194,6 @@ test_expect_success 'log --graph with crossover in octopus merge, no color' '
 	|/
 	* 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_success 'log --graph with crossover in octopus merge with colors' '
@@ -230,7 +221,7 @@ test_expect_success 'log --graph with crossover in octopus merge with colors' '
 '
 
 test_expect_success 'log --graph with crossover in octopus merge and its child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-4 after-merge <<-\EOF
 	* after-4
 	| * after-merge
 	| *---.   octopus-merge
@@ -248,9 +239,6 @@ test_expect_success 'log --graph with crossover in octopus merge and its child,
 	|/
 	* 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_success 'log --graph with crossover in octopus merge and its child with colors' '
@@ -279,7 +267,7 @@ test_expect_success 'log --graph with crossover in octopus merge and its child w
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus tip, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-initial octopus-merge <<-\EOF
 	* after-initial
 	| *---.   octopus-merge
 	| |\ \ \
@@ -296,9 +284,6 @@ test_expect_success 'log --graph with unrelated commit and octopus tip, no color
 	|/
 	* 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' '
@@ -326,7 +311,7 @@ test_expect_success 'log --graph with unrelated commit and octopus tip with colo
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus child, no color' '
-	cat >expect.uncolored <<-\EOF &&
+	test_cmp_graph after-initial after-merge <<-\EOF
 	* after-initial
 	| * after-merge
 	| *---.   octopus-merge
@@ -344,9 +329,6 @@ test_expect_success 'log --graph with unrelated commit and octopus child, no col
 	|/
 	* 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_success 'log --graph with unrelated commit and octopus child with colors' '
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 1d0d3240ff..e1e94176da 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -3,12 +3,11 @@
 test_description='git log --graph of skewed merges'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
 
 check_graph () {
 	cat >expect &&
-	git log --graph --pretty=tformat:%s "$@" >actual.raw &&
-	sed "s/ *$//" actual.raw >actual &&
-	test_cmp expect actual
+	lib_test_cmp_graph --format=%s "$@"
 }
 
 test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC Patch v3 2/2] lib-log-graph: consolidate colored graph cmp logic
  2020-02-24 13:38 ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Abhishek Kumar
@ 2020-02-24 13:38   ` Abhishek Kumar
  2020-02-24 21:17   ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Junio C Hamano
  1 sibling, 0 replies; 18+ messages in thread
From: Abhishek Kumar @ 2020-02-24 13:38 UTC (permalink / raw)
  To: abhishekkumar8222; +Cc: git, johannes.schindelin

Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
---
 t/lib-log-graph.sh           |  6 ++++++
 t/t4202-log.sh               |  4 +---
 t/t4214-log-graph-octopus.sh | 36 ++++++++++++------------------------
 t/t4215-log-skewed-merges.sh |  4 +---
 4 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
index f9c6526eff..1184cceef2 100755
--- a/t/lib-log-graph.sh
+++ b/t/lib-log-graph.sh
@@ -20,3 +20,9 @@ lib_test_cmp_short_graph () {
 	sanitize_log_output >output.sanitized <output &&
 	test_i18ncmp expect output.sanitized
 }
+
+lib_test_cmp_colored_graph () {
+	git log --graph --color=always "$@" >output.colors.raw &&
+	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
+	test_cmp expect.colors output.colors
+}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e025a9cfc2..4694b6d0ce 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -537,9 +537,7 @@ EOF
 
 test_expect_success 'log --graph with merge with log.graphColors' '
 	test_config log.graphColors " blue,invalid-color, cyan, red  , " &&
-	git log --color=always --graph --date-order --pretty=tformat:%s |
-		test_decode_color | sed "s/ *\$//" >actual &&
-	test_cmp expect.colors actual
+	lib_test_cmp_colored_graph --date-order --format=%s
 '
 
 test_expect_success 'log --raw --graph -m with merge' '
diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh
index dedb72ace6..a080325098 100755
--- a/t/t4214-log-graph-octopus.sh
+++ b/t/t4214-log-graph-octopus.sh
@@ -10,6 +10,10 @@ test_cmp_graph () {
 	lib_test_cmp_graph --color=never --date-order --format=%s "$@"
 }
 
+test_cmp_colored_graph () {
+	lib_test_cmp_colored_graph --date-order --format=%s "$@"
+}
+
 test_expect_success 'set up merge history' '
 	test_commit initial &&
 	for i in 1 2 3 4 ; do
@@ -60,9 +64,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 left octopus-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	test_cmp_colored_graph left octopus-merge
 '
 
 # Repeat the previous two tests with "normal" octopus merge (i.e.,
@@ -97,9 +99,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 octopus-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	test_cmp_colored_graph octopus-merge
 '
 
 test_expect_success 'log --graph with normal octopus merge and child, no color' '
@@ -133,9 +133,7 @@ test_expect_success 'log --graph with normal octopus and child 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 after-merge >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	test_cmp_colored_graph after-merge
 '
 
 test_expect_success 'log --graph with tricky octopus merge and its child, no color' '
@@ -171,9 +169,7 @@ test_expect_success 'log --graph with tricky octopus merge and its child with co
 	<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_cmp_colored_graph left after-merge
 '
 
 test_expect_success 'log --graph with crossover in octopus merge, no color' '
@@ -215,9 +211,7 @@ test_expect_success 'log --graph with crossover in octopus merge with colors' '
 	<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_cmp_colored_graph after-4 octopus-merge
 '
 
 test_expect_success 'log --graph with crossover in octopus merge and its child, no color' '
@@ -261,9 +255,7 @@ test_expect_success 'log --graph with crossover in octopus merge and its child w
 	<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_cmp_colored_graph after-4 after-merge
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus tip, no color' '
@@ -305,9 +297,7 @@ test_expect_success 'log --graph with unrelated commit and octopus tip with colo
 	<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_cmp_colored_graph after-initial octopus-merge
 '
 
 test_expect_success 'log --graph with unrelated commit and octopus child, no color' '
@@ -351,9 +341,7 @@ test_expect_success 'log --graph with unrelated commit and octopus child with co
 	<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_cmp_colored_graph after-initial after-merge
 '
 
 test_done
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index e1e94176da..28d0779a8c 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -305,9 +305,7 @@ test_expect_success 'log --graph with multiple tips and colors' '
 	<BLUE>|<RESET><BLUE>/<RESET>
 	* 6_A
 	EOF
-	git log --color=always --graph --date-order --pretty=tformat:%s 6_1 6_3 6_5 >actual.colors.raw &&
-	test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
-	test_cmp expect.colors actual.colors
+	lib_test_cmp_colored_graph --date-order --pretty=tformat:%s 6_1 6_3 6_5
 '
 
 test_expect_success 'log --graph with multiple tips' '
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic
  2020-02-24 13:38 ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Abhishek Kumar
  2020-02-24 13:38   ` [GSoC Patch v3 2/2] lib-log-graph: consolidate colored graph cmp logic Abhishek Kumar
@ 2020-02-24 21:17   ` Junio C Hamano
  1 sibling, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-02-24 21:17 UTC (permalink / raw)
  To: Abhishek Kumar; +Cc: git, johannes.schindelin

Abhishek Kumar <abhishekkumar8222@gmail.com> writes:

> Changes in v3:
> - Elaborate on first commit message.
> - Fix SP violations in lib-log-graph.
>
> Changes in v2:
> - Arrange patches by functions changed instead of files changed.
> - Rename library functions by prepending 'lib_'.
> - Drop outdated sanitization calls to sed.
> - Create specialized helpers to reduce code noise.
> - Fix style violations.

This round looks quite polished.  I'll queue after massaging the log
message of the first step.

Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-02-24 21:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-16 13:47 [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Abhishek Kumar
2020-02-16 13:47 ` [GSoC Patch 2/5] t3430: use lib-log-graph functions Abhishek Kumar
2020-02-19 17:23   ` Junio C Hamano
2020-02-16 13:47 ` [GSoC Patch 3/5] t4215: " Abhishek Kumar
2020-02-19 17:26   ` Junio C Hamano
2020-02-16 13:47 ` [GSoC Patch 4/5] t4214: " Abhishek Kumar
2020-02-19 17:29   ` Junio C Hamano
2020-02-16 13:47 ` [GSoC Patch 5/5] t4202: " Abhishek Kumar
2020-02-19 17:31   ` Junio C Hamano
2020-02-17  0:05 ` [GSoC Patch 1/5] lib-log-graph.sh: consolidate test_cmp_graph logic Junio C Hamano
2020-02-19 17:32   ` Junio C Hamano
2020-02-20  9:15 ` [GSoC PATCH v2 0/2] Consolidate " Abhishek Kumar
2020-02-20  9:15   ` [GSoC PATCH v2 1/2] lib-log-graph: consolidate " Abhishek Kumar
2020-02-20 17:49     ` Junio C Hamano
2020-02-20  9:15   ` [PATCH v2 2/2] lib-log-graph: consolidate colored graph cmp logic Abhishek Kumar
2020-02-24 13:38 ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Abhishek Kumar
2020-02-24 13:38   ` [GSoC Patch v3 2/2] lib-log-graph: consolidate colored graph cmp logic Abhishek Kumar
2020-02-24 21:17   ` [GSoC Patch v3 1/2] lib-log-graph: consolidate test_cmp_graph logic Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).