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

2017-02-22 Thread brian m. carlson 
On Wed, Feb 22, 2017 at 03:42:25PM -0800, Junio C Hamano wrote:
> "brian m. carlson"  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 
> > ---
> >  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?

Okay.  I'll try that in a reroll.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

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

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

> "brian m. carlson"  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 
> > ---
> >  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

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

2017-02-22 Thread Junio C Hamano 
"brian m. carlson"  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 
> ---
>  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?

>
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index d09acfe48e..e6c2a7a369 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -349,4 +349,23 @@ test_expect_success 'diff-tree --stdin with log 
> formatting' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success 'diff-tree --stdin with two parent commits' '
> + cat >expect <<-\EOF &&
> + c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
> + :04 04 da7a33fa77d8066d6698643940ce5860fe2d7fb3 
> f977ed46ae6873c1c30ab878e15a4accedc3618b M  dir
> + :100644 100644 01e79c32a8c99c557f0757da7cb6d65b3414466d 
> f4615da674c09df322d6ba8d6b21ecfb1b1ba510 M  file0
> + :00 100644  
> 7289e35bff32727c08dda207511bec138fdb9ea5 A  file3
> + 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
> + :04 04 847b63d013de168b2de618c5ff9720d5ab27e775 
> 65f5c9dd60ce3b2b3324b618ac7accf8d912c113 M  dir
> + :00 100644  
> b1e67221afe8461efd244b487afca22d46b95eb8 A  file1
> + 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
> + :04 04 da7a33fa77d8066d6698643940ce5860fe2d7fb3 
> 847b63d013de168b2de618c5ff9720d5ab27e775 M  dir
> + :100644 100644 01e79c32a8c99c557f0757da7cb6d65b3414466d 
> b414108e81e5091fe0974a1858b4d0d22b107f70 M  file0
> + :100644 00 01e79c32a8c99c557f0757da7cb6d65b3414466d 
>  D  file2
> + EOF
> + git rev-list --parents master | git diff-tree --stdin >actual &&
> + test_cmp expect actual
> +'
> +
> +
>  test_done