Subject: Re: [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd

From: Richard Hansen <[email-masked]>
Date: Thu, 05 Jun 2014 21:00:07 -0400

On 2014-06-05 17:11, Junio C Hamano wrote:
> Richard Hansen <[email-masked]> writes:
> 
>> Because test_eval_ is defined while zsh is in sh emulation mode, the
>> shell code passed as an argument to test_expect_success would normally
>> be evaluated in sh emulation mode.  However, with this change, it is
>> now possible to evaluate the test code in zsh mode by adding the
>> following line to a zsh-based test script:
>>
>>     emulate -R zsh -c 'test_eval_override () { eval "$*"; }'
>>
>> With test_eval_override defined in zsh emulation mode, the call to
>> test_eval_override from test_eval_ will temporarily cause zsh to
>> switch from sh emulation mode to zsh emulation mode.
> 
> Micronit: aren't all "zsh emulation mode"s above "zsh native mode"s?

Sort of...  Zsh's emulation is under-documented, confusing, and
awkwardly implemented (in my opinion) so it's hard to precisely describe
what's going on in a few words.

I'll explain it here more precisely (apologies for the length) and you
can let me know if I should add more detail to the commit message and/or
code comments:

As far as I understand it, there isn't really such a thing as an
"emulation mode" in Zsh -- the phrase "emulation mode" is just a
convenient way to refer to the mechanism.  Zsh "emulates" other shells
by simply toggling a particular collection of shell options.  For
example, 'emulate sh' turns on the option that controls field splitting
and turns off the option that updates $0 whenever a function is called
(among other option changes).

One might expect 'emulate <shell> -c <code>' to be equivalent to
'emulate <shell>; <code>; emulate <previous_shell>' but it's not --
there's a significant behavior difference.  When '-c <code>' is used,
and only when '-c <code>' is used, a function defined inside <code> gets
"sticky options":  The shell remembers the state of some (all?) options
and saves them along with the function definition.  Whenever the
function is called, those sticky options are temporarily restored for
the duration of the function call.  (This is not entirely accurate --
there are some relatively unimportant cases where the sticky options
aren't restored -- but I don't understand the subtleties of the
mechanism well enough to provide a perfectly accurate description.)

If a function with sticky options calls a function without sticky
options, the sticky options remain in effect -- Zsh doesn't temporarily
revert the sticky options while it calls the non-sticky function.

Thus, if a function foo() is declared inside 'emulate sh -c <code>', and
it calls a function bar() that was declared outside of 'emulate', the
options are still set for POSIX sh during bar()'s execution.  This might
cause bar() to misbehave.  To work around this, bar() must be declared
like 'emulate zsh -c "bar() { stuff... }"' so that bar() gets its own
sticky options.

This commit makes it possible to declare a function that will get sticky
options so that we can control the state of the shell when the scriptlet
is executed.

> 
> In any case, the above explanation confuses me somewhat.  test_eval_
> is fed a scriptlet defined for various test_expect_success tests,
> and they are written in POSIX shells, not zsh, so wouldn't it be
> wrong to run them as if they are zsh native scripts, following
> non-POSIX shell syntax rules?

The scriptlets in lib-prompt-tests.sh are not actually written for POSIX
sh -- they are written in a common subset of the zsh and bash languages
(I should document this in lib-prompt-tests.sh).

We want to test how the __git_ps1 code behaves when interpreted in
"native" zsh mode (default options), because that's how it will be used
in the wild, so the scriptlets must be valid zsh code.  We also want to
test how __git_ps1 behaves in native bash mode, so the scriptlets must
also be valid bash code.  (Fortunately the similarities between the
shells make this easy to do.)

An alternative to this commit -- and I kinda like this idea so I'm
tempted to rewrite the series -- would be to do change the form of the
tests in lib-prompt-tests.sh to something like this:

    test_expect_success 'name of test here' '
        run_in_native_shell_mode '\''
            scriptlet code here
        '\''
    '

Then lib-zsh.sh would do this:

    run_in_native_shell_mode () { emulate -R zsh -c "$*"; }

and lib-bash.sh would do this:

    run_in_native_shell_mode () { eval "$*"; }

This approach makes it clear to others that the scriptlet code is not
actually POSIX shell code, but a common subset of multiple shell languages.

What do you think?

Ignoring t9902 for a moment, we could even stop doing that messy 'exec
$SHELL "$0" "$@"' stuff in lib-*sh.sh and let t9903 and t9904 run under
/bin/sh.  Then run_in_native_shell_mode() would be defined as follows:

  for zsh:

    # wrap the scriptlet in a function in case it does 'return'
    run_in_native_shell_mode () { zsh -c "foo () { $*; }; foo"; }

  for bash:

    # wrap the scriptlet in a function in case it does 'return'
    run_in_native_shell_mode () { bash -c "foo () { $*; }; foo"; }

Running the scriptlets in a separate process like this would cause its
own mess -- the separate processes wouldn't be able to use those
convenience shell functions I added (pcmode_expected(), etc.), so either
there'd be a lot of code copy+paste or each scriptlet would have to
source a helper library.

-Richard

> 
> Puzzled...
> 
>> Signed-off-by: Richard Hansen <[email-masked]>
>> ---
>>  t/test-lib.sh | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index c081668..3779634 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -414,7 +414,12 @@ maybe_setup_valgrind () {
>>  test_eval_ () {
>>  	# This is a separate function because some tests use
>>  	# "return" to end a test_expect_success block early.
>> -	eval </dev/null >&3 2>&4 "$*"
>> +	if command -v test_eval_override >/dev/null 2>&1
>> +	then
>> +		test_eval_override "$*"
>> +	else
>> +		eval "$*"
>> +	fi </dev/null >&3 2>&4
>>  }
>>  
>>  test_run_ () {