threads / patch / 11879

patch, 9 partsUse git diff instead of diff in t7201

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

## tl;dr

8 messages between Feb 4, 2008 and Feb 6, 2008. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Daniel Barkalow· Feb 4, 2008, 18:36 UTC · lore

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

Show changes to t/t7201-co.sh +2 −2

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

Johannes Schindelin· Feb 5, 2008, 01:40 UTC · re: Daniel Barkalow · lore

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

Hi,

On Mon, 4 Feb 2008, Daniel Barkalow wrote:

Show 6 quoted lines

> 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

Daniel Barkalow· Feb 5, 2008, 20:39 UTC · re: Johannes Schindelin · lore

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

On Tue, 5 Feb 2008, Johannes Schindelin wrote:

Show 14 quoted lines

> 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*

Junio C Hamano· Feb 5, 2008, 23:44 UTC · re: Daniel Barkalow · lore

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

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.

Daniel Barkalow· Feb 6, 2008, 01:13 UTC · re: Junio C Hamano · lore

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

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

Show 9 quoted lines

> 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*

Junio C Hamano· Feb 6, 2008, 02:10 UTC · re: Daniel Barkalow · lore

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

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 16 quoted lines

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

Daniel Barkalow· Feb 6, 2008, 04:30 UTC · re: Junio C Hamano · lore

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

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

Show 28 quoted lines

> 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*

Junio C Hamano· Feb 6, 2008, 05:21 UTC · re: Daniel Barkalow · lore

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

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 9 quoted lines

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