From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sun, 06 Sep 2015 09:51:43 GMT
Subject: Re: [PATCH 5/5] test-lib-functions: detect test_when_finished in subshell
Message-ID: <CAPig+cR+_nPFbJmOf3s90oB=Jedg1B3YgOCzSwWa8nwP8-QbeA@mail.gmail.com>
In-Reply-To: <59a8583f79c5ac4661f140262e01cd602286f740.1441458341.git.john@keeping.me.uk>

On Sat, Sep 5, 2015 at 9:12 AM, John Keeping <john@keeping.me.uk> wrote:
> test_when_finished does nothing in a subshell because the change to
> test_cleanup does not affect the parent.
>
> There is no POSIX way to detect that we are in a subshell ($$ and $PPID
> are specified to remain unchanged), but we can detect it on Bash and
> fall back to ignoring the bug on other shells.

I'm not necessarily advocating this, but think it's worth mentioning
that an alternate solution would be to fix test_when_finished() to work
correctly in subshells rather than disallowing its use. This can be done
by having test_when_finished() collect the cleanup actions in a file
rather than in a shell variable.

Pros:
* works in subshells
* portable across all shells (no Bash special-case)
* one less rule (restriction) for test writers to remember

Cons:
* slower
* could interfere with tests expecting very specific 'trash' directory
  contents (but locating this file under .git might make it safe)

> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
>  t/test-lib-functions.sh | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 0e80f37..6dffb8b 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -736,6 +736,11 @@ test_seq () {
>  # what went wrong.
>
>  test_when_finished () {
> +       # We cannot detect when we are in a subshell in general, but by
> +       # doing so on Bash is better than nothing (the test will
> +       # silently pass on other shells).
> +       test "${BASH_SUBSHELL-0}" = 0 ||
> +       error "bug in test script: test_when_finished does nothing in a subshell"
>         test_cleanup="{ $*
>                 } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
>  }
> --
> 2.5.0.466.g9af26fa