From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 08 Mar 2024 22:36:22 GMT
Subject: Re: [PATCH v2] git-difftool--helper: honor `--trust-exit-code` with `--dir-diff`
Message-ID: <xmqqcys4fj15.fsf@gitster.g>
In-Reply-To: <20240308221229.GA1908@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

>> -test_expect_success 'difftool honors exit status if command not found' '
>> -	test_config difftool.nonexistent.cmd i-dont-exist &&
>> -	test_config difftool.trustExitCode false &&
>> -	test_must_fail git difftool -y -t nonexistent branch
>> -'
>> +	test_expect_success "difftool ${opt} honors exit status if command not found" "
>> +		test_config difftool.nonexistent.cmd i-dont-exist &&
>> +		test_config difftool.trustExitCode false &&
>> +		if test "${opt}" = '--dir-diff'
>
> The quoting doesn't quite work here.

Thanks for looking at them carefully.

In general, when you want to interpolate an variable that exists
outside test_expect_success, you should write it this way:

	for var in a "b c"
	do
		test_expect_success "message with $var interpolated" '
			command and "$var" as its argument
		'
	done

The last parameter to test_expect_{success,failure} is eval'ed, so 
enclose it within a pair of single quotes, and let the eval to
interpolate references to $variables at runtime (as opposed to when
the parameters to test_expect_success are formulated) avoids a lot
of surprises and headaches.

Perhaps we should have something like the above as a hint in
t/README?