Subject: Re: [PATCH 3/4] rev-parse test: use test_cmp instead of "test" builtin

From: Junio C Hamano <[email-masked]>
Date: Tue, 03 Sep 2013 13:01:13 -0700

Jonathan Nieder <[email-masked]> writes:

> Use test_cmp instead of passing two command substitutions to the
> "test" builtin.  This way:
>
>  - when tests fail, they can print a helpful diff if run with
>    "--verbose"
>
>  - the argument order "test_cmp expect actual" feels natural,
>    unlike test <known> = <unknown> that seems a little backwards

I do not mind to drop s/a little // here, by the way.

>  - the exit status from invoking git is checked, so if rev-parse
>    starts segfaulting then the test will notice and fail
>
> Use a custom function for this instead of test_cmp_rev to emphasize
> that we are testing the output from "git rev-parse" with certain
> arguments, not checking that the revisions are equal in abstract.
>
> Reported-by: Felipe Contreras <[email-masked]>
> Signed-off-by: Jonathan Nieder <[email-masked]>
> ---
>  t/t6101-rev-parse-parents.sh | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
> index 416067c..8a6ff66 100755
> --- a/t/t6101-rev-parse-parents.sh
> +++ b/t/t6101-rev-parse-parents.sh
> @@ -8,6 +8,12 @@ test_description='Test git rev-parse with different parent options'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-t6000.sh # t6xxx specific functions
>  
> +test_cmp_rev_output () {
> +	git rev-parse --verify "$1" >expect &&
> +	eval "$2" >actual &&
> +	test_cmp expect actual
> +}

After applying this patch and running "git show | grep test_cmp_rev_output",
I notice that the second is always "git rev-parse <something>".  Do
we still need to eval these, or would it be sufficient to do

        test_cmp_rev_output () {
                git rev-parse --verify "$1" >expect &&
                git rev-parse --verify "$2" >actual &&
                test_cmp expect actual
        }

here, and make users like so:

	-	test_cmp_rev_output tags/start "git rev-parse start^0"
	+	test_cmp_rev_output tags/start start^0

Am I missing something?