threads / patch / 15467

patcht7501: always use test_cmp instead of diff

Subject: [PATCH] t7501: always use test_cmp instead of diff

## tl;dr

5 messages between Sep 9, 2008 and Sep 10, 2008. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Miklos Vajna· Sep 9, 2008, 23:41 UTC · lore

This should make the output more readable (by default using diff -u) when some tests fail.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

 t/t7501-commit.sh |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

Show changes to t/t7501-commit.sh +6 −6

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 0edd9dd..6f8c038 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -141,7 +141,7 @@ EOF
 
 test_expect_success \
     'validate git-rev-list output.' \
-    'diff current expected'
+    'test_cmp current expected'
 
 test_expect_success 'partial commit that involves removal (1)' '
 
@@ -151,7 +151,7 @@ test_expect_success 'partial commit that involves removal (1)' '
 	git commit -m "Partial: add elif" elif &&
 	git diff-tree --name-status HEAD^ HEAD >current &&
 	echo "A	elif" >expected &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -160,7 +160,7 @@ test_expect_success 'partial commit that involves removal (2)' '
 	git commit -m "Partial: remove file" file &&
 	git diff-tree --name-status HEAD^ HEAD >current &&
 	echo "D	file" >expected &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -171,7 +171,7 @@ test_expect_success 'partial commit that involves removal (3)' '
 	git commit -m "Partial: modify elif" elif &&
 	git diff-tree --name-status HEAD^ HEAD >current &&
 	echo "M	elif" >expected &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -187,7 +187,7 @@ test_expect_success 'amend commit to fix author' '
 		expected &&
 	git commit --amend --author="$author" &&
 	git cat-file -p HEAD > current &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -256,7 +256,7 @@ test_expect_success 'amend commit to fix author' '
 		expected &&
 	git commit --amend --author="$author" &&
 	git cat-file -p HEAD > current &&
-	diff expected current
+	test_cmp expected current
 
 '

-- 
1.6.0.1

Miklos Vajna· Sep 9, 2008, 23:49 UTC · re: Miklos Vajna · lore

Re: [PATCH] t7501: always use test_cmp instead of diff

On Wed, Sep 10, 2008 at 01:41:06AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:

> This should make the output more readable (by default using diff -u)
> when some tests fail.

Sorry for the duplication, I didn't want to send it twice. I also just realised after sending that I forgot to rebase it again current master, where git-rev-list is replaced by git rev-list, but I guess git am -3 handles it. ;-)

Jeff King· Sep 9, 2008, 23:54 UTC · re: Miklos Vajna · lore

Re: [PATCH] t7501: always use test_cmp instead of diff

On Wed, Sep 10, 2008 at 01:41:06AM +0200, Miklos Vajna wrote:

>  test_expect_success \
>      'validate git-rev-list output.' \
> -    'diff current expected'
> +    'test_cmp current expected'

We seem to use the convention of

  test_cmp <expected> <actual>

elsewhere, rather than

  test_cmp <actual> <expected>

as you have here. Most noticeably, that means the diff will show deviations from expected, rather "what should be done to make this as expected". But it is also possible that in the future test_cmp could be enhanced to do something more interesting.

I don't think it is worth it to go fix all such instances, but while you are here...

-Peff

Junio C Hamano· Sep 10, 2008, 00:02 UTC · re: Jeff King · lore

Re: [PATCH] t7501: always use test_cmp instead of diff

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

> On Wed, Sep 10, 2008 at 01:41:06AM +0200, Miklos Vajna wrote:
>
>>  test_expect_success \
>>      'validate git-rev-list output.' \
>> -    'diff current expected'
>> +    'test_cmp current expected'
>
> We seem to use the convention of
>
>   test_cmp <expected> <actual>
>
> elsewhere, rather than
>
>   test_cmp <actual> <expected>
>
> as you have here.  Most noticeably, that means the diff will show
> deviations from expected, rather "what should be done to make this as
> expected".

Yes, please. I am glad to see somebody noticed it (I've been fixing these whenever I touched vicinity of the ones that had comparisons swapped).

Miklos Vajna· Sep 10, 2008, 17:32 UTC · re: Junio C Hamano · lore

This should make the output more readable (by default using diff -u) when some tests fail.

Also changed the diff order from "current expected" to "expected current".

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Tue, Sep 09, 2008 at 05:02:09PM -0700, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> > We seem to use the convention of
> >
> >   test_cmp <expected> <actual>
> >
> > elsewhere, rather than
> >
> >   test_cmp <actual> <expected>
> >
> > as you have here.  Most noticeably, that means the diff will show
> > deviations from expected, rather "what should be done to make this
> > as expected".
>
> Yes, please.

Here is an updated patch that does this as well.

 t/t7501-commit.sh |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

Show changes to t/t7501-commit.sh +6 −6

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 469bff8..63bfc6d 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -141,7 +141,7 @@ EOF
 
 test_expect_success \
     'validate git rev-list output.' \
-    'diff current expected'
+    'test_cmp expected current'
 
 test_expect_success 'partial commit that involves removal (1)' '
 
@@ -151,7 +151,7 @@ test_expect_success 'partial commit that involves removal (1)' '
 	git commit -m "Partial: add elif" elif &&
 	git diff-tree --name-status HEAD^ HEAD >current &&
 	echo "A	elif" >expected &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -160,7 +160,7 @@ test_expect_success 'partial commit that involves removal (2)' '
 	git commit -m "Partial: remove file" file &&
 	git diff-tree --name-status HEAD^ HEAD >current &&
 	echo "D	file" >expected &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -171,7 +171,7 @@ test_expect_success 'partial commit that involves removal (3)' '
 	git commit -m "Partial: modify elif" elif &&
 	git diff-tree --name-status HEAD^ HEAD >current &&
 	echo "M	elif" >expected &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -187,7 +187,7 @@ test_expect_success 'amend commit to fix author' '
 		expected &&
 	git commit --amend --author="$author" &&
 	git cat-file -p HEAD > current &&
-	diff expected current
+	test_cmp expected current
 
 '
 
@@ -256,7 +256,7 @@ test_expect_success 'amend commit to fix author' '
 		expected &&
 	git commit --amend --author="$author" &&
 	git cat-file -p HEAD > current &&
-	diff expected current
+	test_cmp expected current
 
 '

-- 
1.6.0.1