Re: [PATCH v4 2/4] Add testcases for the --detect-dir-renames diffcore flag.

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 4, 2010, 22:09 UTC
Message-ID: <20101004220923.GA9491@burratino>
In-Reply-To: <20101004213703.GW4983@home.lan>

Yann Dirson wrote:

> On Mon, Oct 04, 2010 at 03:32:41PM -0500, Jonathan Nieder wrote:

>> It might make sense to compute the tree, commit, etc one at a time
>> instead of this long one-liner.
>
> If moved into a function which would make it readable, yes.

Something like

	commit_index () {
		test_tick &&
		tree=$(git write-tree) &&
		commit=$(
			printf "%s\n" "$*" |
			git commit-tree $tree
		) &&
		git update-ref HEAD $commit
	}

? Maybe the following (similar to what you use in later tests) would be even better, for more verbose output when running with -v.

	commit_index () {
		test_tick &&
		git commit -m "$*"
	}

Show 5 quoted lines

>> 	printf "Line %s\n" 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 >a/path0 &&
>> 	...
>
> Well, when seeing for the first time such a construct, i tend to
> wonder how portable it is amont printf implementations.

Yes, it's portable.

Show 6 quoted lines

>> Nit: although compare_diff_patch ensures the result is not dependent
>> on the hash function, these hard-coded hashes are still hard for a
>> human to read.  Could they be computed instead?
>
> Well, that would just make the test harder to read imho.  Using
> regexps would help for readability

If you had said "writability" I would agree with you here. And that's an important concern, too.

What I was suggesting looks like this:

	path0_id2=$(git rev-parse :b/path0) &&
	path100_id=$(git rev-parse :b/path100) &&
	cat >expected <<-EOF &&
	:040000 040000 $zeroes $zeroes R100	a/	b/
	:100644 000000 $path3_id $zeroes D	a/path3
	:100644 100644 $path2_id $path2_id R100	a/path2	b/2path
	:100644 100644 $path0_id $path0_id2 R093	a/path0	b/path0
	:100644 100644 $path1_id $path1_id R100	a/path1	b/path1
	:000000 100644 $zeroes $path100_id A	b/path100
	EOF
	...

Maybe it would be better to do

	cat >expected <<-\EOF &&
	:040000 040000 X X R100	a/	b/
	:100644 000000 X X D	a/path3
	:100644 100644 X X R100	a/path2	b/2path
	:100644 100644 X X R093	a/path0	b/path0
	:100644 100644 X X R100	a/path1	b/path1
	:000000 100644 X X A	b/path100
	EOF

since the hashes are not being checked, anyway.