Re: [PATCH] t: add multi-parent test of diff-tree --stdin

From: Jeff King <peff@peff.net>
Date: Feb 22, 2017, 23:45 UTC
Message-ID: <20170222234535.h7krqqnhwiwllf4v@sigill.intra.peff.net>
In-Reply-To: <xmqqfuj5aj32.fsf@gitster.mtv.corp.google.com>

On Wed, Feb 22, 2017 at 03:42:25PM -0800, Junio C Hamano wrote:

Show 18 quoted lines

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > We were lacking a test that covered the multi-parent case for commits.
> > Add a basic test to ensure we do not regress this behavior in the
> > future.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  t/t4013-diff-various.sh | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > It's a little bit ugly to me that this test hard-codes so many values,
> > and I'm concerned that it may be unnecessarily brittle.  However, I
> > don't have a good idea of how to perform the kind of comprehensive test
> > we need otherwise.
> 
> Hmph, perhaps the expectation can be created out of the output from
> 'git diff-tree master^ master' or something?

I had a similar thought. It may also be worth testing that:

  echo "$(git rev-parse master) $(git rev-parse other)" | git diff-tree --stdin

shows the diff between "other" and "master", and not just "master^" and "master" (i.e., it is not clear from the test expectation that the code actually is parsing the second parent and not accidentally ignoring it).

For completeness, it would probably make sense to test the multi-parent case, too.

-Peff