Re: [PATCH] t5318: avoid unnecessary command substitutions

2018-08-13 Thread Junio C Hamano
SZEDER Gábor  writes:

> - echo $(git -C repo log --pretty="%ct" -1) \
> - $(git -C repo rev-parse one) >expect &&
> + {
> + git -C repo log --pretty=format:"%ct " -1 &&
> + git -C repo rev-parse one
> + } >expect &&

Heh, "format:"%ct " to make the first one end with an incomplete
line?  Neat trick.


Re: [PATCH] t5318: avoid unnecessary command substitutions

2018-08-13 Thread Derrick Stolee

On 8/12/2018 8:30 PM, SZEDER Gábor wrote:

Two tests added in dade47c06c (commit-graph: add repo arg to graph
readers, 2018-07-11) prepare the contents of 'expect' files by
'echo'ing the results of command substitutions.  That's unncessary,
avoid them by directly saving the output of the commands executed in
those command substitutions.

Signed-off-by: SZEDER Gábor 
---
  t/t5318-commit-graph.sh | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 1c148ebf21..3c1ffad491 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -444,25 +444,27 @@ test_expect_success 'setup non-the_repository tests' '
  test_expect_success 'parse_commit_in_graph works for non-the_repository' '
test-tool repository parse_commit_in_graph \
repo/.git repo "$(git -C repo rev-parse two)" >actual &&
-   echo $(git -C repo log --pretty="%ct" -1) \
-   $(git -C repo rev-parse one) >expect &&
+   {
+   git -C repo log --pretty=format:"%ct " -1 &&
+   git -C repo rev-parse one
+   } >expect &&
test_cmp expect actual &&
  
  	test-tool repository parse_commit_in_graph \

repo/.git repo "$(git -C repo rev-parse one)" >actual &&
-   echo $(git -C repo log --pretty="%ct" -1 one) >expect &&
+   git -C repo log --pretty="%ct" -1 one >expect &&
test_cmp expect actual
  '
  
  test_expect_success 'get_commit_tree_in_graph works for non-the_repository' '

test-tool repository get_commit_tree_in_graph \
repo/.git repo "$(git -C repo rev-parse two)" >actual &&
-   echo $(git -C repo rev-parse two^{tree}) >expect &&
+   git -C repo rev-parse two^{tree} >expect &&
test_cmp expect actual &&
  
  	test-tool repository get_commit_tree_in_graph \

repo/.git repo "$(git -C repo rev-parse one)" >actual &&
-   echo $(git -C repo rev-parse one^{tree}) >expect &&
+   git -C repo rev-parse one^{tree} >expect &&
test_cmp expect actual
  '
  
These make sense and are good examples for future test patterns. Thanks, 
Szeder.


Reviewed-by: Derrick Stolee 


[PATCH] t5318: avoid unnecessary command substitutions

2018-08-12 Thread SZEDER Gábor
Two tests added in dade47c06c (commit-graph: add repo arg to graph
readers, 2018-07-11) prepare the contents of 'expect' files by
'echo'ing the results of command substitutions.  That's unncessary,
avoid them by directly saving the output of the commands executed in
those command substitutions.

Signed-off-by: SZEDER Gábor 
---
 t/t5318-commit-graph.sh | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 1c148ebf21..3c1ffad491 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -444,25 +444,27 @@ test_expect_success 'setup non-the_repository tests' '
 test_expect_success 'parse_commit_in_graph works for non-the_repository' '
test-tool repository parse_commit_in_graph \
repo/.git repo "$(git -C repo rev-parse two)" >actual &&
-   echo $(git -C repo log --pretty="%ct" -1) \
-   $(git -C repo rev-parse one) >expect &&
+   {
+   git -C repo log --pretty=format:"%ct " -1 &&
+   git -C repo rev-parse one
+   } >expect &&
test_cmp expect actual &&
 
test-tool repository parse_commit_in_graph \
repo/.git repo "$(git -C repo rev-parse one)" >actual &&
-   echo $(git -C repo log --pretty="%ct" -1 one) >expect &&
+   git -C repo log --pretty="%ct" -1 one >expect &&
test_cmp expect actual
 '
 
 test_expect_success 'get_commit_tree_in_graph works for non-the_repository' '
test-tool repository get_commit_tree_in_graph \
repo/.git repo "$(git -C repo rev-parse two)" >actual &&
-   echo $(git -C repo rev-parse two^{tree}) >expect &&
+   git -C repo rev-parse two^{tree} >expect &&
test_cmp expect actual &&
 
test-tool repository get_commit_tree_in_graph \
repo/.git repo "$(git -C repo rev-parse one)" >actual &&
-   echo $(git -C repo rev-parse one^{tree}) >expect &&
+   git -C repo rev-parse one^{tree} >expect &&
test_cmp expect actual
 '
 
-- 
2.18.0.408.g42635c01bc