Re: [PATCH 1/2] t6101: add a test for rev-parse $garbage^@
- From
Jeff King <peff@peff.net>
- Date
- May 23, 2018, 22:12 UTC
- Message-ID
- <20180523221249.GC32171@sigill.intra.peff.net>
- In-Reply-To
- <20180523204613.11333-1-newren@gmail.com>
On Wed, May 23, 2018 at 01:46:12PM -0700, Elijah Newren wrote:
Show 11 quoted lines
> 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