From: Derrick Stolee <stolee@gmail.com>
Date: Mon, 13 Aug 2018 11:23:18 GMT
Subject: Re: [PATCH] t5318: avoid unnecessary command substitutions
Message-ID: <9e934039-6b97-28bd-b5c9-34ac5f6754bb@gmail.com>
In-Reply-To: <20180813003010.17610-1-szeder.dev@gmail.com>

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 <szeder.dev@gmail.com>
> ---
>   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 <dstolee@microsoft.com>