Re: [PATCH 1/2] t6101: add a test for rev-parse $garbage^@

[Jeff King <peff@peff.net>]

On Wed, May 23, 2018 at 01:46:12PM -0700, Elijah Newren wrote:

> diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
> index 8c617981a3..7b1b2dbdf2 100755
> --- a/t/t6101-rev-parse-parents.sh
> +++ b/t/t6101-rev-parse-parents.sh
> @@ -214,4 +214,8 @@ test_expect_success 'rev-list merge^-1x (garbage after ^-1)' '
>  	test_must_fail git rev-list merge^-1x
>  '
>  
> +test_expect_failure 'rev-parse $garbage^@ should not segfault' '
> +	git rev-parse ffffffffffffffffffffffffffffffffffffffff^@
> +'

Two small nits. :)

It may just be me, but for a trivial test+fix like this, I'd rather see
them in the same commit (both for reviewing, and when I'm digging in the
history later).

The second nit is that we may want to use something a little more
symbolic and easier to read here. Thirty-nine f's behaves quite
differently than forty. And eventually we'd like to move away from
having hard-coded commit ids anyway (this is obviously a fake one, but
the length may end up changing).

Perhaps "git rev-parse $EMPTY_TREE^@", which triggers the same bug?

-Peff