From 1a9a23e35c1ba75187ee600fb7a1f107a2fb1893 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 5 Sep 2015 14:12:45 +0100 Subject: [PATCH 1/5] t7610: don't use test_config in a subshell test_config uses test_when_finished to reset the configuration after the test, but this does not work inside a subshell. This does not cause a problem here because the first thing the next test does is to set this config variable itself, but we are about to add a check that will complain when test_when_finished is used in a subshell. In this case, "subdir" not a submodule so test_config has the same effect when run at the top level and can simply be moved out of the subshell. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t7610-mergetool.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 7eeb207b32..6f12b235b3 100755 --- 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 ) && From c545bc6266e36fdb798cf79fd1f7ef9f39791a94 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 5 Sep 2015 14:12:46 +0100 Subject: [PATCH 2/5] t5801: don't use test_when_finished in a subshell test_when_finished has no effect in a subshell. Since the cmp_marks function is only used once, inline it at its call site and move the test_when_finished invocation to the start of the test. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index c9d3ed14c3..362b1581e0 100755 --- 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 ) ' From 5fafc07fca933be10c82ba97a6fd3b28d6b3a02e Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 5 Sep 2015 14:12:47 +0100 Subject: [PATCH 3/5] test-lib-functions: support "test_config -C ..." If used in a subshell, test_config cannot unset variables at the end of a test. This is a problem when testing submodules because we do not want to "cd" at to top level of a test script in order to run the command inside the submodule. Add a "-C" option to test_config (and test_unconfig) so that test_config can be kept outside subshells and still affect subrepositories. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index e8d3c0fdbc..0e80f377ce 100644 --- 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 () { From da568b66f1b481998bee3a2569739ca1302fd65c Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 5 Sep 2015 14:12:48 +0100 Subject: [PATCH 4/5] t7800: don't use test_config in a subshell Use the new "-C" option to test_config to change the configuration in the submodule from the top level of the test so that it can be unset correctly when the test finishes. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index ea35a0241c..48c6e2bc83 100755 --- 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 From 0968f12a99c4ac784b6b7f858003662cfaae117f Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 5 Sep 2015 14:12:49 +0100 Subject: [PATCH 5/5] test-lib-functions: detect test_when_finished in subshell 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. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- 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 0e80f377ce..6dffb8bcde 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" }