[PATCH 8/9] Use git diff instead of diff in t7201

[PATCH 8/9] Use git diff instead of diff in t7201

[Daniel Barkalow]

If the test failed, it was giving really unclear ed script
output. Instead, give a diff that sort of suggests the problem.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 t/t7201-co.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 73d8a00..c6f93a9 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -143,7 +143,7 @@ test_expect_success "checkout -m with dirty tree, renamed" '
 
 	git checkout -m renamer &&
 	fill 1 3 4 5 7 8 >expect &&
-	diff expect uno &&
+	git diff expect uno &&
 	! test -f one &&
 	git diff --cached >current &&
 	! test -s current
@@ -168,7 +168,7 @@ test_expect_success 'checkout -m with merge conflict' '
 	git diff master:one :3:uno |
 	sed -e "1,/^@@/d" -e "/^ /d" -e "s/^-/d/" -e "s/^+/a/" >current &&
 	fill d2 aT d7 aS >expect &&
-	diff current expect &&
+	git diff current expect &&
 	git diff --cached two >current &&
 	! test -s current
 '
-- 
1.5.4.rc3.4.g16335

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> If the test failed, it was giving really unclear ed script
> output. Instead, give a diff that sort of suggests the problem.

Hmm.  I'd actually prefer using "diff -u" instead.

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Daniel Barkalow]

On Fri, 25 Jan 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > If the test failed, it was giving really unclear ed script
> > output. Instead, give a diff that sort of suggests the problem.
> 
> Hmm.  I'd actually prefer using "diff -u" instead.

Should I change the 4 places that currently use "git diff" on a pair of 
filenames to "diff -u"? All 6 places should match, surely.

	-Daniel
*This .sig left intentionally blank*

[PATCH 8/9] Use git diff instead of diff in t7201

[Daniel Barkalow]

If the test failed, it was giving really unclear ed script
output. Instead, give a diff that sort of suggests the problem.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 t/t7201-co.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 73d8a00..c6f93a9 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -143,7 +143,7 @@ test_expect_success "checkout -m with dirty tree, renamed" '
 
 	git checkout -m renamer &&
 	fill 1 3 4 5 7 8 >expect &&
-	diff expect uno &&
+	git diff expect uno &&
 	! test -f one &&
 	git diff --cached >current &&
 	! test -s current
@@ -168,7 +168,7 @@ test_expect_success 'checkout -m with merge conflict' '
 	git diff master:one :3:uno |
 	sed -e "1,/^@@/d" -e "/^ /d" -e "s/^-/d/" -e "s/^+/a/" >current &&
 	fill d2 aT d7 aS >expect &&
-	diff current expect &&
+	git diff current expect &&
 	git diff --cached two >current &&
 	! test -s current
 '
-- 
1.5.4

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Johannes Schindelin]

Hi,

On Mon, 4 Feb 2008, Daniel Barkalow wrote:

> If the test failed, it was giving really unclear ed script
> output. Instead, give a diff that sort of suggests the problem.
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  t/t7201-co.sh |    4 ++--

This is not really a part of the series leading to builtin checkout.  But 
as a standalone patch, I like it (obviously... I tried to push for more 
--no-index uses in the test suite).

Ciao,
Dscho

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Daniel Barkalow]

On Tue, 5 Feb 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Mon, 4 Feb 2008, Daniel Barkalow wrote:
> 
> > If the test failed, it was giving really unclear ed script
> > output. Instead, give a diff that sort of suggests the problem.
> > 
> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> > ---
> >  t/t7201-co.sh |    4 ++--
> 
> This is not really a part of the series leading to builtin checkout.  But 
> as a standalone patch, I like it (obviously... I tried to push for more 
> --no-index uses in the test suite).

Well, it's something I tripped over when builtin-checkout wasn't passing 
the tests and I couldn't figure out what it was doing wrong from the 
output. Like [3/9], it's relevant to evaluating whether the series works, 
even if it's not important for whether it actually does work.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> Well, it's something I tripped over when builtin-checkout wasn't passing 
> the tests and I couldn't figure out what it was doing wrong from the 
> output. Like [3/9], it's relevant to evaluating whether the series works, 
> even if it's not important for whether it actually does work.

Yeah, using "diff -u" instead of just "diff" is an improvement
for debuggability which matters a lot in the test scripts.

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Daniel Barkalow]

On Tue, 5 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Well, it's something I tripped over when builtin-checkout wasn't passing 
> > the tests and I couldn't figure out what it was doing wrong from the 
> > output. Like [3/9], it's relevant to evaluating whether the series works, 
> > even if it's not important for whether it actually does work.
> 
> Yeah, using "diff -u" instead of just "diff" is an improvement
> for debuggability which matters a lot in the test scripts.

I think I asked before and didn't hear back (or maybe I got distracted 
and didn't ask); do you want this to use "diff -u" everywhere or "git 
diff" everywhere? This test is currently part "diff" and part "git diff", 
and I went for git diff" everywhere, but you seemed to prefer "diff -u".

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Tue, 5 Feb 2008, Junio C Hamano wrote:
>
>> Daniel Barkalow <barkalow@iabervon.org> writes:
>> 
>> > Well, it's something I tripped over when builtin-checkout wasn't passing 
>> > the tests and I couldn't figure out what it was doing wrong from the 
>> > output. Like [3/9], it's relevant to evaluating whether the series works, 
>> > even if it's not important for whether it actually does work.
>> 
>> Yeah, using "diff -u" instead of just "diff" is an improvement
>> for debuggability which matters a lot in the test scripts.
>
> I think I asked before and didn't hear back (or maybe I got distracted 
> and didn't ask); do you want this to use "diff -u" everywhere or "git 
> diff" everywhere? This test is currently part "diff" and part "git diff", 
> and I went for git diff" everywhere, but you seemed to prefer "diff -u".

Typically the test sequence is "do this with git, do that with
git, produce the output with git, now what happened?  did we
produce a correct result?"  And we often compare 'expect' and
'actual' to see if there are discrepancies.

My preference is NEVER using "git diff" when comparing expected
result and the actual output from git.  When "git diff" has
breakage, it would break unrelated tests and make debugging
needlessly harder.

We need to have tests for regressions in "git diff", but such a
test would use "git diff" in earlier parts of the test sequence
that _produce_ the output to be compared with expected result.

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Daniel Barkalow]

On Tue, 5 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Tue, 5 Feb 2008, Junio C Hamano wrote:
> >
> >> Daniel Barkalow <barkalow@iabervon.org> writes:
> >> 
> >> > Well, it's something I tripped over when builtin-checkout wasn't passing 
> >> > the tests and I couldn't figure out what it was doing wrong from the 
> >> > output. Like [3/9], it's relevant to evaluating whether the series works, 
> >> > even if it's not important for whether it actually does work.
> >> 
> >> Yeah, using "diff -u" instead of just "diff" is an improvement
> >> for debuggability which matters a lot in the test scripts.
> >
> > I think I asked before and didn't hear back (or maybe I got distracted 
> > and didn't ask); do you want this to use "diff -u" everywhere or "git 
> > diff" everywhere? This test is currently part "diff" and part "git diff", 
> > and I went for git diff" everywhere, but you seemed to prefer "diff -u".
> 
> Typically the test sequence is "do this with git, do that with
> git, produce the output with git, now what happened?  did we
> produce a correct result?"  And we often compare 'expect' and
> 'actual' to see if there are discrepancies.
> 
> My preference is NEVER using "git diff" when comparing expected
> result and the actual output from git.  When "git diff" has
> breakage, it would break unrelated tests and make debugging
> needlessly harder.

Certainly, although we seem to do a lousy job of ordering tests currently 
such that the tests that fail are the ones for the thing that's broken; it 
took a lot of work to get builtin-checkout to the point to reaching 
t7201-co.sh, and by that point it just about all worked. In any case, I'll 
go for "diff -u" all around in that test, since I'm changing it.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 8/9] Use git diff instead of diff in t7201

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Tue, 5 Feb 2008, Junio C Hamano wrote:
> ...
>> My preference is NEVER using "git diff" when comparing expected
>> result and the actual output from git.  When "git diff" has
>> breakage, it would break unrelated tests and make debugging
>> needlessly harder.
>
> Certainly, although we seem to do a lousy job of ordering tests currently 
> such that the tests that fail are the ones for the thing that's broken;

Yeah, Porcelains have become we take for granted ;-).