All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Yann Dirson <ydirson@free.fr>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v4 2/4] Add testcases for the --detect-dir-renames diffcore flag.
Date: Mon, 4 Oct 2010 17:09:23 -0500	[thread overview]
Message-ID: <20101004220923.GA9491@burratino> (raw)
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 "$*"
	}

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

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

  reply	other threads:[~2010-10-04 22:12 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-03 20:42 [RFC PATCH v4 0/4] Detection of directory renames Yann Dirson
2010-10-03 20:42 ` [PATCH v4 1/4] Introduce wholesame directory move detection in diffcore Yann Dirson
2010-10-03 20:42   ` [PATCH v4 2/4] Add testcases for the --detect-dir-renames diffcore flag Yann Dirson
2010-10-03 20:42     ` [PATCH v4 3/4] Allow hiding renames of individual files involved in a directory rename Yann Dirson
2010-10-03 20:42       ` [PATCH v4 4/4] Add testcases for the --hide-dir-rename-details diffcore flag Yann Dirson
2010-10-03 23:04         ` Sverre Rabbelier
2010-10-03 23:06       ` [PATCH v4 3/4] Allow hiding renames of individual files involved in a directory rename Sverre Rabbelier
2010-10-03 23:28         ` Junio C Hamano
2010-10-04  6:43           ` Sverre Rabbelier
2010-10-04 18:21         ` Yann Dirson
2010-10-04  3:03     ` [PATCH v4 2/4] Add testcases for the --detect-dir-renames diffcore flag Ævar Arnfjörð Bjarmason
2010-10-04 18:32       ` Yann Dirson
2010-10-04 20:32     ` Jonathan Nieder
2010-10-04 21:37       ` Yann Dirson
2010-10-04 22:09         ` Jonathan Nieder [this message]
2010-10-05  9:21         ` Andreas Ericsson
2010-10-04  2:59   ` [PATCH v4 1/4] Introduce wholesame directory move detection in diffcore Ævar Arnfjörð Bjarmason
2010-10-04 18:19     ` Yann Dirson
2010-10-04  7:28   ` Jonathan Nieder
2010-10-04 21:13     ` Yann Dirson
2010-10-05  1:06   ` Jonathan Nieder
2010-10-06 23:13     ` Yann Dirson
2010-10-04  6:20 ` [RFC PATCH v4 0/4] Detection of directory renames Jonathan Nieder
2010-10-05  1:42 ` Jonathan Nieder
2010-10-06 23:17   ` Yann Dirson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101004220923.GA9491@burratino \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=ydirson@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.