A recent thread [0] made me realise that using test_when_finished in a
subshell is likely to be a bug, since the change to $test_cleanup will
be lost when the subshell exits.

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
gracefully fall back to disabling the test on other shells, which is
what the patch below does.

There are three tests currently in master that fail with this patch (at
least on my system):

        Test Summary Report
        -------------------
        t7610-mergetool.sh                               (Wstat: 256 Tests: 18 
Failed: 1)
          Failed test:  7
          Non-zero exit status: 1
        t7800-difftool.sh                                (Wstat: 256 Tests: 56 
Failed: 1)
          Failed test:  56
          Non-zero exit status: 1
        t5801-remote-helpers.sh                          (Wstat: 256 Tests: 30 
Failed: 1)
          Failed test:  27
          Non-zero exit status: 1

All are harmless at the moment and t7610 and t5801 can be fixed by
moving the test_when_finished call out of the subshell relatively
easily.

t7800 (in its final test) calls test_config in a subshell which has cd'd
into a submodule.

Is this something worth worrying about, or is it sufficiently rare that
we can live with the current behaviour?

[0] http://article.gmane.org/gmane.comp.version-control.git/277199

-- >8 --
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e8d3c0f..d29cd7b 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -722,6 +722,8 @@ test_seq () {
 # what went wrong.
 
 test_when_finished () {
+       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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to