Re: [PATCH 2/3] Add a test for git-rerere

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 21, 2006, 08:34 UTC
Message-ID: <Pine.LNX.4.63.0612210927150.19693@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7v4prqfm6c.fsf@assigned-by-dhcp.cox.net>

Hi,

On Wed, 20 Dec 2006, Junio C Hamano wrote:

Show 13 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +sha1=4f58849a60b4f969a2848966b6d02893b783e8fb
> > +rr=.git/rr-cache/$sha1
> > +test_expect_success 'recorded preimage' "grep ======= $rr/preimage"
> > +
> > +test_expect_success 'no postimage or thisimage yet' \
> > +	"test ! -f $rr/postimage -a ! -f $rr/thisimage"
> 
> I am worried that this test specifies the behaviour a bit too
> precisely.  It is not t0000 basic where we want bit-for-bit
> match with the expected result.  Wouldn't this test break when
> you further improve xdl_merge, for example?

In this case, I doubt that xdl_merge() can do _anything_. The example is set up so it _must_ fail with any sane merge algorithm.

And I very much wanted the behaviour put down very precisely, so that I catch any bugs in builtin-rerere. In fact, as I went, I wrote the test with the original rerere, and then whipped the builtin until it conformed.

Since anyone modifying rerere's behaviour can modify this test to catch 
that behaviour, too, I am not so worried.

> > +rm $rr/postimage
> > +echo -ne "$sha1\ta1\0" > .git/rr-cache/MERGE_RR
> 
> Hmmmmmm.  I've stayed away from "echo -e" so far...

Rather do it with tr? Like this:

	echo -n "$sha1	a1X" | tr X '\0' > .git/rr-cache/MERGE_RR

Show 6 quoted lines

> > +sha2=4000000000000000000000000000000000000000
> > +rr2=.git/rr-cache/$sha2
> > +mkdir $rr2
> > +echo Hello > $rr2/preimage
> 
> Collisions are not so likely and perhaps we do not care.

Both. If collisions were likely, we both would know by now _and_ would be in serious trouble. And we do not care, since there cannot be any other directory in rr-cache, apart from $sha1/.

Show 10 quoted lines

> > +case "$(date -d @11111111 +%s 2>/dev/null)" in
> > +[1-9]*)
> > +	# it is a recent GNU date. good.
> > +	now=$(date +%s)
> > +	let almost_15_days_ago=$now+60-15*86400
> > +	let just_over_15_days_ago=$now-1-15*86400
> > +	let almost_60_days_ago=$now+60-60*86400
> > +	let just_over_60_days_ago=$now-1-60*86400
> 
> Hmmmmm.  I've stayed away from "let" so far as well...

Is $((1+2)) portable enough? I always thought this is a bashism...

Ciao, Dscho