| Message ID | 8e950ddfba3fa0f6d0551a153228548da6af6117.1573520653.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t4215: don't put git commands upstream of pipe | expand |
Denton Liu <liu.denton@gmail.com> writes: > A little late to the party but since this cleanup hasn't been done yet, > let's do it now. We can apply this patch to the tip of > 'jc/log-graph-simplify'. > ... > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && Obviously good but I wonder if show_graph () { git log --graph --pretty=tformat:%s >actual.raw && sed "s/ *$//" actual.raw && rm actual.raw } would help to make it even more readable without too much repetition. Will queue; thanks. > test_cmp expect actual > ' > > @@ -68,7 +69,8 @@ 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 && > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && > test_cmp expect actual > ' > > @@ -99,7 +101,8 @@ 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 && > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && > test_cmp expect actual > ' > > @@ -139,7 +142,8 @@ 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 && > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && > test_cmp expect actual > ' > > @@ -175,7 +179,8 @@ 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 && > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && > test_cmp expect actual > ' > > @@ -210,7 +215,8 @@ 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 && > > - git log --graph --date-order --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --date-order --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && > test_cmp expect actual > ' > > @@ -250,7 +256,8 @@ 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 && > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > + git log --graph --pretty=tformat:%s >actual.raw && > + sed "s/ *$//" actual.raw >actual && > test_cmp expect actual > '
On Tue, Nov 12, 2019 at 03:57:08PM +0900, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > A little late to the party but since this cleanup hasn't been done yet, > > let's do it now. We can apply this patch to the tip of > > 'jc/log-graph-simplify'. > > ... > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > Obviously good but I wonder if > > show_graph () { > git log --graph --pretty=tformat:%s >actual.raw && > sed "s/ *$//" actual.raw && > rm actual.raw > } > > would help to make it even more readable without too much repetition. I think it would indeed, but then let's go one step further, and add that 'test_cmp expect actual' to the function, too, and call it 'check_graph'. > Will queue; thanks. > > > test_cmp expect actual > > ' > > > > @@ -68,7 +69,8 @@ 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 && > > > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > test_cmp expect actual > > ' > > > > @@ -99,7 +101,8 @@ 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 && > > > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > test_cmp expect actual > > ' > > > > @@ -139,7 +142,8 @@ 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 && > > > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > test_cmp expect actual > > ' > > > > @@ -175,7 +179,8 @@ 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 && > > > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > test_cmp expect actual > > ' > > > > @@ -210,7 +215,8 @@ 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 && > > > > - git log --graph --date-order --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --date-order --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > test_cmp expect actual > > ' > > > > @@ -250,7 +256,8 @@ 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 && > > > > - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && > > + git log --graph --pretty=tformat:%s >actual.raw && > > + sed "s/ *$//" actual.raw >actual && > > test_cmp expect actual > > '
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh index d33c6438d8..0a2555fb28 100755 --- a/t/t4215-log-skewed-merges.sh +++ b/t/t4215-log-skewed-merges.sh @@ -31,7 +31,8 @@ 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 && - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual ' @@ -68,7 +69,8 @@ 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 && - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual ' @@ -99,7 +101,8 @@ 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 && - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual ' @@ -139,7 +142,8 @@ 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 && - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual ' @@ -175,7 +179,8 @@ 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 && - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual ' @@ -210,7 +215,8 @@ 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 && - git log --graph --date-order --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --date-order --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual ' @@ -250,7 +256,8 @@ 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 && - git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual && + git log --graph --pretty=tformat:%s >actual.raw && + sed "s/ *$//" actual.raw >actual && test_cmp expect actual '
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. 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. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Thanks for the suggestion, Gábor. (Is that how I should refer to you? I recently learned that some poeple write their names in ALL CAPS as a convention.) A little late to the party but since this cleanup hasn't been done yet, let's do it now. We can apply this patch to the tip of 'jc/log-graph-simplify'. t/t4215-log-skewed-merges.sh | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)