Subject: Re: [PATCH 3/9] git p4 test: simplify quoting involving TRASH_DIRECTORY

From: Johannes Sixt <[email-masked]>
Date: Tue, 26 Jun 2012 08:24:06 +0200

Am 6/26/2012 3:18, schrieb Pete Wyckoff:
>  test_expect_success 'exit when p4 fails to produce marshaled output' '
> -	badp4dir="$TRASH_DIRECTORY/badp4dir" &&
> -	mkdir "$badp4dir" &&
> -	test_when_finished "rm \"$badp4dir/p4\" && rmdir \"$badp4dir\"" &&
> -	cat >"$badp4dir"/p4 <<-EOF &&
> +	mkdir badp4dir &&
> +	test_when_finished "rm badp4dir/p4 && rmdir badp4dir" &&
> +	cat >badp4dir/p4 <<-EOF &&
>  	#!$SHELL_PATH
>  	exit 1
>  	EOF
> -	chmod 755 "$badp4dir"/p4 &&
> -	PATH="$badp4dir:$PATH" git p4 clone --dest="$git" //depot >errs 2>&1 ; retval=$? &&
> +	chmod 755 badp4dir/p4 &&
> +	PATH="$TRASH_DIRECTORY/badp4dir:$PATH" git p4 clone --dest="$git" //depot >errs 2>&1 ; retval=$? &&
>  	test $retval -eq 1 &&

The long line here is severly broken, because the semicolon breaks the &&
chain; retval would be assigned to even if one of the earlier commands
fails, and that you don't want to treat as success. The least that is
needed is to put the line in braces. But I suggest to rewrite the two
lines above as

	(
		PATH="$TRASH_DIRECTORY/badp4dir:$PATH" &&
		export PATH &&
		test_expect_code 1 git p4 clone --dest="$git" //depot >errs 2>&1
	) &&

>  	test_must_fail grep -q Traceback errs

We don't expect that grep fails due to segfault or something. Write this
line as

	! grep Traceback errs

Also drop the -q; if the test detects a failure, you do want to see the
grep output in a verbose test run.

-- Hannes