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

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Jun 26, 2012, 06:24 UTC
Message-ID: <4FE95586.8090803@viscovery.net>
In-Reply-To: <1340673505-10551-4-git-send-email-pw@padd.com>

Am 6/26/2012 3:18, schrieb Pete Wyckoff:

Show 16 quoted lines

>  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