Merge branch 'jk/test-lint-forbid-when-finished-in-subshell'

Because "test_when_finished" in our test framework queues the
clean-up tasks to be done in a shell variable, it should not be
used inside a subshell.  Add a mechanism to allow 'bash' to catch
such uses, and fix the ones that were found.

* jk/test-lint-forbid-when-finished-in-subshell:
test-lib-functions: detect test_when_finished in subshell
t7800: don't use test_config in a subshell
test-lib-functions: support "test_config -C <dir> ..."
t5801: don't use test_when_finished in a subshell
t7610: don't use test_config in a subshell

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index c9d3ed14c3a3238208b44ce815e37123298dea77..362b1581e092826400d78515cdc8d5cb0f7cfc29 100755 (executable)
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -242,13 +242,6 @@ clean_mark () {
        sort >$(basename "$1")
 }
 
-cmp_marks () {
-       test_when_finished "rm -rf git.marks testgit.marks" &&
-       clean_mark ".git/testgit/$1/git.marks" &&
-       clean_mark ".git/testgit/$1/testgit.marks" &&
-       test_cmp git.marks testgit.marks
-}
-
 test_expect_success 'proper failure checks for fetching' '
        (cd local &&
        test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git fetch 2>error &&
@@ -258,12 +251,15 @@ test_expect_success 'proper failure checks for fetching' '
 '
 
 test_expect_success 'proper failure checks for pushing' '
+       test_when_finished "rm -rf local/git.marks local/testgit.marks" &&
        (cd local &&
        git checkout -b crash master &&
        echo crash >>file &&
        git commit -a -m crash &&
        test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all &&
-       cmp_marks origin
+       clean_mark ".git/testgit/origin/git.marks" &&
+       clean_mark ".git/testgit/origin/testgit.marks" &&
+       test_cmp git.marks testgit.marks
        )
 '
 

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 7eeb207b32b48041037488f42e645b8b12742690..6f12b235b3a9330497e814d5e8f564aec94d0c44 100755 (executable)
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -174,9 +174,9 @@ test_expect_success 'mergetool skips autoresolved' '
 '
 
 test_expect_success 'mergetool merges all from subdir' '
+       test_config rerere.enabled false &&
        (
                cd subdir &&
-               test_config rerere.enabled false &&
                test_must_fail git merge master &&
                ( yes "r" | git mergetool ../submod ) &&
                ( yes "d" "d" | git mergetool --no-prompt ) &&

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index ea35a0241c201ac12c29603a21421199dd097600..48c6e2bc830e74ab8d242e49c08f38609b5ef585 100755 (executable)
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -492,12 +492,12 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' '
 
 test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
        git submodule add ./. submod/ule &&
+       test_config -C submod/ule diff.tool checktrees &&
+       test_config -C submod/ule difftool.checktrees.cmd '\''
+               test -d "$LOCAL" && test -d "$REMOTE" && echo good
+               '\'' &&
        (
                cd submod/ule &&
-               test_config diff.tool checktrees &&
-               test_config difftool.checktrees.cmd '\''
-                       test -d "$LOCAL" && test -d "$REMOTE" && echo good
-               '\'' &&
                echo good >expect &&
                git difftool --tool=checktrees --dir-diff HEAD~ >actual &&
                test_cmp expect actual

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e8d3c0fdbc76d93ea18f6da1c869fc963e7ca81d..6dffb8bcde83b82fa40b05dd8ee815f4e6a3cdd3 100644 (file)
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -201,7 +201,14 @@ test_chmod () {
 
 # Unset a configuration variable, but don't fail if it doesn't exist.
 test_unconfig () {
-       git config --unset-all "$@"
+       config_dir=
+       if test "$1" = -C
+       then
+               shift
+               config_dir=$1
+               shift
+       fi
+       git ${config_dir:+-C "$config_dir"} config --unset-all "$@"
        config_status=$?
        case "$config_status" in
        5) # ok, nothing to unset
@@ -213,8 +220,15 @@ test_unconfig () {
 
 # Set git config, automatically unsetting it after the test is over.
 test_config () {
-       test_when_finished "test_unconfig '$1'" &&
-       git config "$@"
+       config_dir=
+       if test "$1" = -C
+       then
+               shift
+               config_dir=$1
+               shift
+       fi
+       test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$1'" &&
+       git ${config_dir:+-C "$config_dir"} config "$@"
 }
 
 test_config_global () {
@@ -722,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"
 }