git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "James Coglan" <jcoglan@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH v3] t4215: use helper function to check output
Date: Tue, 12 Nov 2019 10:56:22 -0800	[thread overview]
Message-ID: <86af5739a4e5f5f38b2597a86d03d5501ad5468b.1573584933.git.liu.denton@gmail.com> (raw)
In-Reply-To: <8e950ddfba3fa0f6d0551a153228548da6af6117.1573520653.git.liu.denton@gmail.com>

When git commands are placed in the upstream of a pipe, their return
codes are lost. In this particular case, it is especially bad since we
are testing the intricacies of `git log --graph` behavior and if we hit
an unexpected failure or segfault, we want to know this.

Extract the common output checking logic into check_graph() where we
redirect the output of git commands upstream of pipe into a file and
have sed read from that file so that git failures are detected.

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

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Thanks for the feedback, everyone. I decided to take Gábor's suggestion
and write the check_graph() helper function.

Unfortunately, even though in practice I was moving the here-doc lines
down, the diff shows up as me moving the commit and checkout lines up
and this might affect how the blame ends up looking. Is there any way to
force git to make the diff show up the other way in both this diff and
in the blame or is this the best that we can do?

Thanks!

 t/t4215-log-skewed-merges.sh | 208 ++++++++++++++++-------------------
 1 file changed, 97 insertions(+), 111 deletions(-)

diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index d33c6438d8..18709a723e 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -4,8 +4,25 @@ 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_expect_success 'log --graph with merge fusing with its left and right neighbors' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan _p &&
+	test_commit A &&
+	test_commit B &&
+	git checkout -b _q @^ && test_commit C &&
+	git checkout -b _r @^ && test_commit D &&
+	git checkout _p && git merge --no-ff _q _r -m E &&
+	git checkout _r && test_commit F &&
+	git checkout _p && git merge --no-ff _r -m G &&
+	git checkout @^^ && git merge --no-ff _p -m H &&
+
+	check_graph <<-\EOF
 	*   H
 	|\
 	| *   G
@@ -20,23 +37,20 @@ test_expect_success 'log --graph with merge fusing with its left and right neigh
 	|/
 	* A
 	EOF
-
-	git checkout --orphan _p &&
-	test_commit A &&
-	test_commit B &&
-	git checkout -b _q @^ && test_commit C &&
-	git checkout -b _r @^ && test_commit D &&
-	git checkout _p && git merge --no-ff _q _r -m E &&
-	git checkout _r && test_commit F &&
-	git checkout _p && git merge --no-ff _r -m G &&
-	git checkout @^^ && git merge --no-ff _p -m H &&
-
-	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_expect_success 'log --graph with left-skewed merge' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan 0_p && test_commit 0_A &&
+	git checkout -b 0_q 0_p && test_commit 0_B &&
+	git checkout -b 0_r 0_p &&
+	test_commit 0_C &&
+	test_commit 0_D &&
+	git checkout -b 0_s 0_p && test_commit 0_E &&
+	git checkout -b 0_t 0_p && git merge --no-ff 0_r^ 0_s -m 0_F &&
+	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
 	*-----.   0_H
 	|\ \ \ \
 	| | | | * 0_G
@@ -57,23 +71,20 @@ test_expect_success 'log --graph with left-skewed merge' '
 	|/
 	* 0_A
 	EOF
-
-	git checkout --orphan 0_p && test_commit 0_A &&
-	git checkout -b 0_q 0_p && test_commit 0_B &&
-	git checkout -b 0_r 0_p &&
-	test_commit 0_C &&
-	test_commit 0_D &&
-	git checkout -b 0_s 0_p && test_commit 0_E &&
-	git checkout -b 0_t 0_p && git merge --no-ff 0_r^ 0_s -m 0_F &&
-	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 &&
-
-	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_expect_success 'log --graph with nested left-skewed merge' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan 1_p &&
+	test_commit 1_A &&
+	test_commit 1_B &&
+	test_commit 1_C &&
+	git checkout -b 1_q @^ && test_commit 1_D &&
+	git checkout 1_p && git merge --no-ff 1_q -m 1_E &&
+	git checkout -b 1_r @~3 && test_commit 1_F &&
+	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
 	*   1_H
 	|\
 	| *   1_G
@@ -88,23 +99,24 @@ test_expect_success 'log --graph with nested left-skewed merge' '
 	|/
 	* 1_A
 	EOF
-
-	git checkout --orphan 1_p &&
-	test_commit 1_A &&
-	test_commit 1_B &&
-	test_commit 1_C &&
-	git checkout -b 1_q @^ && test_commit 1_D &&
-	git checkout 1_p && git merge --no-ff 1_q -m 1_E &&
-	git checkout -b 1_r @~3 && test_commit 1_F &&
-	git checkout 1_p && git merge --no-ff 1_r -m 1_G &&
-	git checkout @^^ && git merge --no-ff 1_p -m 1_H &&
-
-	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_expect_success 'log --graph with nested left-skewed merge following normal merge' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan 2_p &&
+	test_commit 2_A &&
+	test_commit 2_B &&
+	test_commit 2_C &&
+	git checkout -b 2_q @^^ &&
+	test_commit 2_D &&
+	test_commit 2_E &&
+	git checkout -b 2_r @^ && test_commit 2_F &&
+	git checkout 2_q &&
+	git merge --no-ff 2_r -m 2_G &&
+	git merge --no-ff 2_p^ -m 2_H &&
+	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
 	*   2_K
 	|\
 	| *   2_J
@@ -124,27 +136,23 @@ test_expect_success 'log --graph with nested left-skewed merge following normal
 	|/
 	* 2_A
 	EOF
-
-	git checkout --orphan 2_p &&
-	test_commit 2_A &&
-	test_commit 2_B &&
-	test_commit 2_C &&
-	git checkout -b 2_q @^^ &&
-	test_commit 2_D &&
-	test_commit 2_E &&
-	git checkout -b 2_r @^ && test_commit 2_F &&
-	git checkout 2_q &&
-	git merge --no-ff 2_r -m 2_G &&
-	git merge --no-ff 2_p^ -m 2_H &&
-	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 &&
-
-	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_expect_success 'log --graph with nested right-skewed merge following left-skewed merge' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan 3_p &&
+	test_commit 3_A &&
+	git checkout -b 3_q &&
+	test_commit 3_B &&
+	test_commit 3_C &&
+	git checkout -b 3_r @^ &&
+	test_commit 3_D &&
+	git checkout 3_q && git merge --no-ff 3_r -m 3_E &&
+	git checkout 3_p && git merge --no-ff 3_q -m 3_F &&
+	git checkout 3_r && test_commit 3_G &&
+	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
 	*   3_J
 	|\
 	| *   3_H
@@ -161,26 +169,21 @@ test_expect_success 'log --graph with nested right-skewed merge following left-s
 	|/
 	* 3_A
 	EOF
-
-	git checkout --orphan 3_p &&
-	test_commit 3_A &&
-	git checkout -b 3_q &&
-	test_commit 3_B &&
-	test_commit 3_C &&
-	git checkout -b 3_r @^ &&
-	test_commit 3_D &&
-	git checkout 3_q && git merge --no-ff 3_r -m 3_E &&
-	git checkout 3_p && git merge --no-ff 3_q -m 3_F &&
-	git checkout 3_r && test_commit 3_G &&
-	git checkout 3_p && git merge --no-ff 3_r -m 3_H &&
-	git checkout @^^ && git merge --no-ff 3_p -m 3_J &&
-
-	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_expect_success 'log --graph with right-skewed merge following a left-skewed one' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan 4_p &&
+	test_commit 4_A &&
+	test_commit 4_B &&
+	test_commit 4_C &&
+	git checkout -b 4_q @^^ && test_commit 4_D &&
+	git checkout -b 4_r 4_p^ && git merge --no-ff 4_q -m 4_E &&
+	git checkout -b 4_s 4_p^^ &&
+	git merge --no-ff 4_r -m 4_F &&
+	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
 	*   4_H
 	|\
 	| *   4_G
@@ -198,24 +201,25 @@ test_expect_success 'log --graph with right-skewed merge following a left-skewed
 	|/
 	* 4_A
 	EOF
-
-	git checkout --orphan 4_p &&
-	test_commit 4_A &&
-	test_commit 4_B &&
-	test_commit 4_C &&
-	git checkout -b 4_q @^^ && test_commit 4_D &&
-	git checkout -b 4_r 4_p^ && git merge --no-ff 4_q -m 4_E &&
-	git checkout -b 4_s 4_p^^ &&
-	git merge --no-ff 4_r -m 4_F &&
-	git merge --no-ff 4_p -m 4_G &&
-	git checkout @^^ && git merge --no-ff 4_s -m 4_H &&
-
-	git log --graph --date-order --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_expect_success 'log --graph with octopus merge with column joining its penultimate parent' '
-	cat >expect <<-\EOF &&
+	git checkout --orphan 5_p &&
+	test_commit 5_A &&
+	git branch 5_q &&
+	git branch 5_r &&
+	test_commit 5_B &&
+	git checkout 5_q && test_commit 5_C &&
+	git checkout 5_r && test_commit 5_D &&
+	git checkout 5_p &&
+	git merge --no-ff 5_q 5_r -m 5_E &&
+	git checkout 5_q && test_commit 5_F &&
+	git checkout -b 5_s 5_p^ &&
+	git merge --no-ff 5_p 5_q -m 5_G &&
+	git checkout 5_r &&
+	git merge --no-ff 5_s -m 5_H &&
+
+	check_graph <<-\EOF
 	*   5_H
 	|\
 	| *-.   5_G
@@ -234,24 +238,6 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
 	|/
 	* 5_A
 	EOF
-
-	git checkout --orphan 5_p &&
-	test_commit 5_A &&
-	git branch 5_q &&
-	git branch 5_r &&
-	test_commit 5_B &&
-	git checkout 5_q && test_commit 5_C &&
-	git checkout 5_r && test_commit 5_D &&
-	git checkout 5_p &&
-	git merge --no-ff 5_q 5_r -m 5_E &&
-	git checkout 5_q && test_commit 5_F &&
-	git checkout -b 5_s 5_p^ &&
-	git merge --no-ff 5_p 5_q -m 5_G &&
-	git checkout 5_r &&
-	git merge --no-ff 5_s -m 5_H &&
-
-	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
-	test_cmp expect actual
 '
 
 test_done
-- 
2.24.0.300.g722ba42680


  parent reply	other threads:[~2019-11-12 18:56 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 16:13 [PATCH 00/11] Improve the readability of log --graph output James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 01/11] graph: automatically track visible width of `strbuf` James Coglan via GitGitGadget
2019-10-10 21:07   ` Johannes Schindelin
2019-10-10 23:05     ` Denton Liu
2019-10-11  0:49       ` Derrick Stolee
2019-10-11  1:42       ` Junio C Hamano
2019-10-11  5:01         ` Denton Liu
2019-10-11 16:02           ` Johannes Schindelin
2019-10-11 17:20             ` James Coglan
2019-10-12  0:27               ` Junio C Hamano
2019-10-12 16:22                 ` Johannes Schindelin
2019-10-14 12:55                 ` James Coglan
2019-10-14 13:01                   ` James Coglan
2019-10-16  2:15                   ` Junio C Hamano
2019-10-11  1:40     ` Junio C Hamano
2019-10-11 17:08     ` James Coglan
2019-10-10 16:13 ` [PATCH 02/11] graph: reuse `find_new_column_by_commit()` James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 03/11] graph: reduce duplication in `graph_insert_into_new_columns()` James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 04/11] graph: remove `mapping_idx` and `graph_update_width()` James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 05/11] graph: extract logic for moving to GRAPH_PRE_COMMIT state James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 06/11] graph: tidy up display of left-skewed merges James Coglan via GitGitGadget
2019-10-10 17:19   ` Derrick Stolee
2019-10-11 16:50     ` James Coglan
2019-10-12  1:36       ` Derrick Stolee
2019-10-14 13:11         ` James Coglan
2019-10-10 16:13 ` [PATCH 07/11] graph: commit and post-merge lines for " James Coglan via GitGitGadget
2019-10-10 17:49   ` Derrick Stolee
2019-10-11 17:04     ` James Coglan
2019-10-13  6:56       ` Jeff King
2019-10-14 15:38         ` James Coglan
2019-10-14 17:41           ` Derrick Stolee
2019-10-14 20:42           ` Johannes Schindelin
2019-10-10 16:13 ` [PATCH 08/11] graph: rename `new_mapping` to `old_mapping` James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 09/11] graph: smooth appearance of collapsing edges on commit lines James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 10/11] graph: flatten edges that join to their right neighbor James Coglan via GitGitGadget
2019-10-10 16:13 ` [PATCH 11/11] graph: fix coloring of octopus dashes James Coglan via GitGitGadget
2019-10-10 18:16   ` Denton Liu
2019-10-10 18:28     ` Denton Liu
2019-10-13  7:22     ` Jeff King
2019-10-10 17:54 ` [PATCH 00/11] Improve the readability of log --graph output Derrick Stolee
2019-10-13  7:15 ` Jeff King
2019-10-14 15:49   ` James Coglan
2019-10-15 23:40 ` [PATCH v2 00/13] " James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 01/13] graph: automatically track display width of graph lines James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 02/13] graph: handle line padding in `graph_next_line()` James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 03/13] graph: reuse `find_new_column_by_commit()` James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 04/13] graph: reduce duplication in `graph_insert_into_new_columns()` James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 05/13] graph: remove `mapping_idx` and `graph_update_width()` James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 06/13] graph: extract logic for moving to GRAPH_PRE_COMMIT state James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 07/13] graph: example of graph output that can be simplified James Coglan via GitGitGadget
2019-10-15 23:40   ` [PATCH v2 08/13] graph: tidy up display of left-skewed merges James Coglan via GitGitGadget
2019-10-15 23:41   ` [PATCH v2 09/13] graph: commit and post-merge lines for " James Coglan via GitGitGadget
2019-10-15 23:41   ` [PATCH v2 10/13] graph: rename `new_mapping` to `old_mapping` James Coglan via GitGitGadget
2019-10-15 23:41   ` [PATCH v2 11/13] graph: smooth appearance of collapsing edges on commit lines James Coglan via GitGitGadget
2019-10-15 23:41   ` [PATCH v2 12/13] graph: flatten edges that fuse with their right neighbor James Coglan via GitGitGadget
2019-10-15 23:41   ` [PATCH v2 13/13] graph: fix coloring of octopus dashes James Coglan via GitGitGadget
2019-10-15 23:47   ` [PATCH v3 00/13] Improve the readability of log --graph output James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 01/13] graph: automatically track display width of graph lines James Coglan via GitGitGadget
2019-10-16  3:35       ` Junio C Hamano
2019-10-16  5:10         ` Junio C Hamano
2019-10-15 23:47     ` [PATCH v3 02/13] graph: handle line padding in `graph_next_line()` James Coglan via GitGitGadget
2019-10-16  3:37       ` Junio C Hamano
2019-10-15 23:47     ` [PATCH v3 03/13] graph: reuse `find_new_column_by_commit()` James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 04/13] graph: reduce duplication in `graph_insert_into_new_columns()` James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 05/13] graph: remove `mapping_idx` and `graph_update_width()` James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 06/13] graph: extract logic for moving to GRAPH_PRE_COMMIT state James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 07/13] graph: example of graph output that can be simplified James Coglan via GitGitGadget
2019-10-17 12:30       ` Derrick Stolee
2019-10-18 15:21       ` SZEDER Gábor
2019-11-12  1:08         ` [PATCH] t4215: don't put git commands upstream of pipe Denton Liu
2019-11-12  6:57           ` Junio C Hamano
2019-11-12 10:54             ` SZEDER Gábor
2019-11-12 18:56           ` Denton Liu [this message]
2019-11-13  2:05             ` [PATCH v3] t4215: use helper function to check output Junio C Hamano
2019-10-15 23:47     ` [PATCH v3 08/13] graph: tidy up display of left-skewed merges James Coglan via GitGitGadget
2019-10-16  4:00       ` Junio C Hamano
2019-10-17 12:34         ` Derrick Stolee
2019-10-18  0:49           ` Junio C Hamano
2019-10-15 23:47     ` [PATCH v3 09/13] graph: commit and post-merge lines for " James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 10/13] graph: rename `new_mapping` to `old_mapping` James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 11/13] graph: smooth appearance of collapsing edges on commit lines James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 12/13] graph: flatten edges that fuse with their right neighbor James Coglan via GitGitGadget
2019-10-15 23:47     ` [PATCH v3 13/13] graph: fix coloring of octopus dashes James Coglan via GitGitGadget

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86af5739a4e5f5f38b2597a86d03d5501ad5468b.1573584933.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jcoglan@gmail.com \
    --cc=szeder.dev@gmail.com \
    /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.
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).