Re: [PATCH 5/5] test-lib-functions: detect test_when_finished in subshell

[John Keeping <john@keeping.me.uk>]

On Sun, Sep 06, 2015 at 05:51:43AM -0400, Eric Sunshine wrote:
> 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)

Another con is that we have to worry about the working directory, and
since we can't reliably detect if we're in a subshell every cleanup
action probably has to be something like:

	( cd '$(pwd)' && $* )

It's certainly possible but it adds another bit of complexity.

Since there are only 3 out of over 13,000 tests that use this
functionality (and it's quite easy to change them not to) I'm not sure
it's worth supporting it.

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