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

[Jonathan Nieder <jrnieder@gmail.com>]

Junio C Hamano wrote:

> 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?

I was tempted to use test_cmp_rev, which would have the same effect.
The original test checked the output of "git rev-parse start^0", while
the test as modified above checks the output of "git rev-parse
--verify start^0".

I slightly prefer the version without --verify because "git rev-parse
--verify" is well exercised elsewhere in the testsuite (but then, so
is rev-parse without --verify, so it's not a big deal).

Abbreviating as follows

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

would also be fine with me.  All it would take is a good replacement
for the placeholder "foo".  The added flexibility of "compare a rev
to the output of an arbitrary command" doesn't get used.

Jonathan