Re: [PATCH 2/3] t9010-svn-fi: Add tests for svn-fi

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 2, 2011, 03:18 UTC
Message-ID: <20110202031812.GB15285@elie>
In-Reply-To: <20110202024944.GA11339@kytes>

Ramkumar Ramachandra wrote:

> Jonathan Nieder writes:
>> Ramkumar Ramachandra wrote:

>> Is it impossible to test without svn present or otherwise speed
>> these up?  Some random ideas:

[...]

Show 8 quoted lines

> Hm, I'm not entirely convinced -- the final verdict is always dictated
> by whether or not the emitted dumpstream loads. Many streams that look
> alright to the eye don't actually load because of small intricacies
> like missing newlines. It'll be awfully complicated reverse-engineer
> all these intricate rules and write them in terms of sed/ awk
> commands. Yes, svnrdump will be faster, but it'll only be available
> with the later versions of Subversion (same problem with fixing
> svnadmin bottlenecks).

If svnrdump is fast enough, wouldn't something like

 if svnrdump --help >/dev/null 2>&1
 then
	load_dump () {
		svnrdump load "file://$1"
	}
	test_set_prereq SVN
 elif svnadmin --help >/dev/null 2>&1
 then
	load_dump () {
		svnadmin load "$1"
	}
	test_set_prereq SVN
 else
	: no usable svn installation
 fi

do it?

Show 5 quoted lines

>>> +svn_look () {
>>> +	subcommand=$1 &&
>>> +	shift &&
>>> +	svnlook "$subcommand" "$svnrepo" "$@"
>>> +}

[...]

> Just a convinience: I am using svn_look to look at the properties of
> nodes in several tests.

Ah, I missed the implicit $svnrepo argument. Makes sense then (might be good to have a comment to explain the purpose, though).

Show 18 quoted lines

>>> +test_expect_success 'normal empty files' '
>>> +	reinit_svn &&
>>> +	cat >expect.tree <<-\EOF &&
>>> +	/
>>> +	 foo
>>> +	 bar
>>> +	EOF
>>> +	cat >input <<-\EOF &&
>>> +	reset refs/heads/master
>>> +	commit refs/heads/master
>>> +	mark :1
>>> +	author nobody <nobody@localhost> 1170199019 +0100
>>> +	committer nobody <nobody@localhost> 1170199019 +0100
>>
>> Where do the dates come from?  Why should I (the reader) expect
>> them?
>
> What do you suggest? I need some valid "generic" timestamp + offset.

	test_tick
	...
	author nobody <nobody@localhost> $GIT_AUTHOR_DATE

[...]

> Okay, I'll try this. Is there no other way? Can we whip up a shell
> script to convert the timestamp?

Maybe the standard "date" utility can help with conversions?

I'm not sure what kinds of validation you're doing on the dates, hence the vague answers. For examples of fuzzing out the unimportant bits,

 git grep fuzz t/*.sh

can help.

>>> +test_expect_success 'malformed fast-import stream: author 2' '

[...]

Show 9 quoted lines

>>> +	try_load input must_fail
>> 
>> What's wrong with this stream?
>
> author nobody <localhost>
>                         ^
> Parse error there, since build_svn_author tries to use the part of the
> email address appearing before the '@'. This is just a temporary test
> -- we should make the svn_author generation logic configurable.

So the stream is not malformed? In that case, I guess the intent is

 test_expect_failure 'does something reasonable with email address without @' '
	...
	try_load input
 '

Thanks again. Jonathan