On Fri, Mar 08, 2024 at 02:36:22PM -0800, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@xxxxxxxxx> 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?
Makes sense. I've sent a separate patch series addressing this issue in
[1].
Thanks!
Patrick
[1]: https://lore.kernel.org/git/cover.1711028473.git.ps@xxxxxx/
Attachment:
signature.asc
Description: PGP signature
