Subject: Re: [PATCH v2 2/6] test-reach: add run_three_modes method

From: SZEDER Gábor <[email-masked]>
Date: Tue, 18 Sep 2018 20:02:00 +0200

On Mon, Sep 17, 2018 at 09:08:44PM -0700, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email-masked]>
> 
> The 'test_three_modes' method assumes we are using the 'test-tool
> reach' command for our test. However, we may want to use the data
> shape of our commit graph and the three modes (no commit-graph,
> full commit-graph, partial commit-graph) for other git commands.
> 
> Split test_three_modes to be a simple translation on a more general
> run_three_modes method that executes the given command and tests
> the actual output to the expected output.
> 
> While inspecting this code, I realized that the final test for
> 'commit_contains --tag' is silently dropping the '--tag' argument.
> It should be quoted to include both.

Nit: while quoting the function's arguments does fix the issue, it
leaves the tests prone to the same issue in the future.  Wouldn't it
be better to use $@ inside the function to refer to all its arguments?

> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  t/t6600-test-reach.sh | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
> index d139a00d1d..1b18e12a4e 100755
> --- a/t/t6600-test-reach.sh
> +++ b/t/t6600-test-reach.sh
> @@ -53,18 +53,22 @@ test_expect_success 'setup' '
>  	git config core.commitGraph true
>  '
>  
> -test_three_modes () {
> +run_three_modes () {
>  	test_when_finished rm -rf .git/objects/info/commit-graph &&
> -	test-tool reach $1 <input >actual &&
> +	$1 <input >actual &&
>  	test_cmp expect actual &&
>  	cp commit-graph-full .git/objects/info/commit-graph &&
> -	test-tool reach $1 <input >actual &&
> +	$1 <input >actual &&
>  	test_cmp expect actual &&
>  	cp commit-graph-half .git/objects/info/commit-graph &&
> -	test-tool reach $1 <input >actual &&
> +	$1 <input >actual &&
>  	test_cmp expect actual
>  }
>  
> +test_three_modes () {
> +	run_three_modes "test-tool reach $1"
> +}
> +
>  test_expect_success 'ref_newer:miss' '
>  	cat >input <<-\EOF &&
>  	A:commit-5-7
> @@ -219,7 +223,7 @@ test_expect_success 'commit_contains:hit' '
>  	EOF
>  	echo "commit_contains(_,A,X,_):1" >expect &&
>  	test_three_modes commit_contains &&
> -	test_three_modes commit_contains --tag
> +	test_three_modes "commit_contains --tag"
>  '
>  
>  test_expect_success 'commit_contains:miss' '
> -- 
> gitgitgadget
>