All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Richard Hansen <rhansen@bbn.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd
Date: Fri, 06 Jun 2014 09:53:20 -0700	[thread overview]
Message-ID: <xmqqiooeq8sf.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <53911297.9030106@bbn.com> (Richard Hansen's message of "Thu, 05 Jun 2014 21:00:07 -0400")

Richard Hansen <rhansen@bbn.com> writes:

> On 2014-06-05 17:11, Junio C Hamano wrote:
> ...
>> 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.)

OK.  The above all makes sense, but I think we would prefer a
solution with changes limited to lib-prompt-tests and lib-zsh
without touching lib-test-functions at all if that is the case.

> 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
>         '\''
>     '

Yeah, or even:

	prompt_test_expect success 'name of test' '
        	scriptlet code here
	'

with a helper prompt_test_expect that wraps whatever logic you will
have in run-in-native-shell-mode.

> ...
> 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:

No, let's not go there (and you stated the reason why we do not want
to yourself already ;-).

  reply	other threads:[~2014-06-06 16:53 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-19 22:55 [PATCH] git-prompt.sh: don't assume the shell expands the value of PS1 Richard Hansen
2014-05-20 18:38 ` Junio C Hamano
2014-05-27  7:40   ` [PATCH 00/10] Zsh prompt tests Richard Hansen
2014-05-27  7:40     ` [PATCH 01/10] t9903: remove Zsh test from the suite of Bash " Richard Hansen
2014-05-27  7:40     ` [PATCH 02/10] t9903: put the Bash pc mode prompt test cases in a function Richard Hansen
2014-05-27  7:40     ` [PATCH 03/10] t9903: move test name prefix to a separate variable Richard Hansen
2014-05-27  7:40     ` [PATCH 04/10] t9903: run pc mode tests again with PS1 expansion disabled Richard Hansen
2014-05-27  7:40     ` [PATCH 05/10] t9903: include "Bash" in test names via new $shellname var Richard Hansen
2014-05-27  7:40     ` [PATCH 06/10] t9903: move PS1 color code variable definitions to lib-bash.sh Richard Hansen
2014-05-27  7:40     ` [PATCH 07/10] t9903: move prompt tests to a new lib-prompt-tests.sh file Richard Hansen
2014-05-27  7:40     ` [PATCH 08/10] lib-prompt-tests.sh: put all tests inside a function Richard Hansen
2014-05-27  7:40     ` [PATCH 09/10] lib-prompt-tests.sh: add variable for string that encodes percent in PS1 Richard Hansen
2014-05-27  7:41     ` [PATCH 10/10] t9904: new __git_ps1 tests for Zsh Richard Hansen
2014-05-29 19:02       ` Thomas Rast
2014-05-29 22:30         ` [PATCH 11/10] fixup! " Richard Hansen
2014-06-04 21:01     ` [PATCH v2 00/11] Zsh prompt tests Richard Hansen
2014-06-04 21:01       ` [PATCH v2 01/11] t9903: remove Zsh test from the suite of Bash " Richard Hansen
2014-06-04 21:01       ` [PATCH v2 02/11] t9903: put the Bash pc mode prompt test cases in a function Richard Hansen
2014-06-04 21:01       ` [PATCH v2 03/11] t9903: move test name prefix to a separate variable Richard Hansen
2014-06-04 21:01       ` [PATCH v2 04/11] t9903: run pc mode tests again with PS1 expansion disabled Richard Hansen
2014-06-04 21:01       ` [PATCH v2 05/11] t9903: include "Bash" in test names via new $shellname var Richard Hansen
2014-06-04 21:01       ` [PATCH v2 06/11] t9903: move PS1 color code variable definitions to lib-bash.sh Richard Hansen
2014-06-04 21:01       ` [PATCH v2 07/11] t9903: move prompt tests to a new lib-prompt-tests.sh file Richard Hansen
2014-06-04 21:01       ` [PATCH v2 08/11] lib-prompt-tests.sh: put all tests inside a function Richard Hansen
2014-06-04 21:01       ` [PATCH v2 09/11] lib-prompt-tests.sh: add variable for string that encodes percent in PS1 Richard Hansen
2014-06-04 21:01       ` [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd Richard Hansen
2014-06-05 21:11         ` Junio C Hamano
2014-06-06  1:00           ` Richard Hansen
2014-06-06 16:53             ` Junio C Hamano [this message]
2014-06-04 21:01       ` [PATCH v2 11/11] t9904: new __git_ps1 tests for Zsh Richard Hansen
2014-06-10 20:06       ` [PATCH v2 00/11] Zsh prompt tests Torsten Bögershausen
2014-06-10 20:28         ` Richard Hansen
2014-06-11  1:16           ` brian m. carlson
2014-06-11 15:27             ` Richard Hansen
2014-06-11 23:46               ` brian m. carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqiooeq8sf.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rhansen@bbn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.