Re: [PATCH v3 09/10] Fix tests breaking when checkout path contains shell metacharacters

[Bryan Donlan <bdonlan@fushizen.net>]

On Mon, May 05, 2008 at 09:04:08AM +0200, Johannes Sixt wrote:
> Bryan Donlan schrieb:
> > This fixes the remainder of the issues where the test script itself is at
> > fault for failing when the git checkout path contains whitespace or other
> > shell metacharacters.
> 
> Maybe a note along these lines would be in order:
> 
> The majority of git svn tests used the idiom
> 
>   test_expect_success "title" "test script using $svnrepo"
> 
> These were changed to have the test script in single-quotes:
> 
>   test_expect_success "title" 'test script using "$svnrepo"'
> 
> (which makes the patch so large). One consequence of this change is that
> in the verbose test output the value of $svnrepo (and in some cases other
> variables, too) is no longer expanded, i.e. previously we saw
> 
>   * expecting success:
> 	test script using /path/to/git/t/trash/svnrepo
> 
> but now it is:
> 
>   * expecting success:
> 	test script using "$svnrepo"

Ah, good point. I don't see an easy way to make them show in expanded
form without breaking space handling, unfortunately, but if a v4 is
needed I can toss that in.

> 
> 
> > 
> > Signed-off-by: Bryan Donlan <bdonlan@fushizen.net>
> 
> >  test_expect_success 'push with insteadOf' '
> >  	mk_empty &&
> > -	TRASH=$(pwd)/ &&
> > -	git config url.$TRASH.insteadOf trash/ &&
> > +	TRASH="$(pwd)/" &&
> 
> This one would not be necessary, but I don't mind.

I wasn't sure what the rules were regarding non-copy-assignments, and
wanted to play it safe (there was a conflict in this area when rebasing
too).

> > -test_expect_success 'make sure r2 still has old file' '
> > +test_expect_success 'make sure r2 still has old file' "
> >  	cd x &&
> > -		test -n "$(git svn find-rev r1)" &&
> > -		git reset --hard $(git svn find-rev r1) &&
> > +		test -n \"\$(git svn find-rev r1)\" &&
> > +		git reset --hard \$(git svn find-rev r1) &&
> >  		test -f README &&
> >  		test ! -f FOLLOWME &&
> > -		test x$(git svn find-rev r2) = x
> > -'
> > +		test x\$(git svn find-rev r2) = x
> > +"
> 
> Why did you change the quoting from sq to dq here?

Erk, missed this in git add -i processing :) I'll drop this hunk.

> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> > index 3dc261d..ae7082b 100755
> > --- a/t/t9500-gitweb-standalone-no-errors.sh
> > +++ b/t/t9500-gitweb-standalone-no-errors.sh
> > @@ -10,6 +10,7 @@ commandline, and checks that it would not write any errors
> >  or warnings to log.'
> >  
> >  gitweb_init () {
> > +	safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
> 
> How portable is this? (Just asking...)

A grovel through perl-5.8.8's source indicates it has a fallback when
the platform does not have a real getcwd() function
(util.c:Perl_getcwd_sv(), called from ext/POSIX/POSIX.xs)

> 
> -- Hannes
> 
>