git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] t7501: always use test_cmp instead of diff
@ 2008-09-09 23:37 Miklos Vajna
  0 siblings, 0 replies; 6+ messages in thread
From: Miklos Vajna @ 2008-09-09 23:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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(-)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] t7501: always use test_cmp instead of diff
@ 2008-09-09 23:41 Miklos Vajna
  2008-09-09 23:49 ` Miklos Vajna
  2008-09-09 23:54 ` Jeff King
  0 siblings, 2 replies; 6+ messages in thread
From: Miklos Vajna @ 2008-09-09 23:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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(-)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] t7501: always use test_cmp instead of diff
  2008-09-09 23:41 Miklos Vajna
@ 2008-09-09 23:49 ` Miklos Vajna
  2008-09-09 23:54 ` Jeff King
  1 sibling, 0 replies; 6+ messages in thread
From: Miklos Vajna @ 2008-09-09 23:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

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. ;-)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] t7501: always use test_cmp instead of diff
  2008-09-09 23:41 Miklos Vajna
  2008-09-09 23:49 ` Miklos Vajna
@ 2008-09-09 23:54 ` Jeff King
  2008-09-10  0:02   ` Junio C Hamano
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff King @ 2008-09-09 23:54 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] t7501: always use test_cmp instead of diff
  2008-09-09 23:54 ` Jeff King
@ 2008-09-10  0:02   ` Junio C Hamano
  2008-09-10 17:32     ` Miklos Vajna
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2008-09-10  0:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Miklos Vajna, git

Jeff King <peff@peff.net> writes:

> 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).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] t7501: always use test_cmp instead of diff
  2008-09-10  0:02   ` Junio C Hamano
@ 2008-09-10 17:32     ` Miklos Vajna
  0 siblings, 0 replies; 6+ messages in thread
From: Miklos Vajna @ 2008-09-10 17:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

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:
> > 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(-)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-09-10 17:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-09 23:37 [PATCH] t7501: always use test_cmp instead of diff Miklos Vajna
  -- strict thread matches above, loose matches on Subject: below --
2008-09-09 23:41 Miklos Vajna
2008-09-09 23:49 ` Miklos Vajna
2008-09-09 23:54 ` Jeff King
2008-09-10  0:02   ` Junio C Hamano
2008-09-10 17:32     ` Miklos Vajna

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).