From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 14 Nov 2010 17:41:33 GMT
Subject: Re: [PATCH 3/4] tests: use test_cmp instead of piping to diff(1)
Message-ID: <20101114174133.GD26459@burratino>
In-Reply-To: <1289745857-16704-4-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason wrote:

> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -37,9 +37,10 @@ head1=$(add_file sm1 foo1 foo2)
>  test_expect_success 'added submodule' "
>  	git add sm1 &&
>  	git diff-index -p --submodule=log HEAD >actual &&
> -	diff actual - <<-EOF
> +	cat >expected <<-EOF &&
>  Submodule sm1 0000000...$head1 (new submodule)
>  EOF
> +	test_cmp expected actual
>  "

Nice.  Reminds me of
http://thread.gmane.org/gmane.comp.version-control.git/160168/focus=160222
(use of "cmp - foo.expect" in git svn tests); anyone want to take a
stab at fixing that, too?

> --- a/t/t7401-submodule-summary.sh
> +++ b/t/t7401-submodule-summary.sh
[...]
>  test_expect_success '--for-status' "
>      git submodule summary --for-status HEAD^ >actual &&
> -    test_cmp actual - <<EOF
> +	test_cmp actual - <<EOF

I think this tab should be spaces?  Also, shouldn't this be

	cat >expect <<EOF &&
	...
	EOF
	test_cmp expect actual

to match the others?  There are a few similar cases in the patch.

I assume you've checked the patched tests still pass.  So with the exception
just mentioned,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.