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

From: Johannes Sixt <[email-masked]>
Date: Mon, 05 May 2008 09:04:08 +0200

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"

> 
> Signed-off-by: Bryan Donlan <[email-masked]>

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

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

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

-- Hannes