Skip to content

Commit 4cb5488

Browse files
committed
Merge branch 'jk/test-lint-forbid-when-finished-in-subshell' into maint
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
2 parents bc49712 + 0968f12 commit 4cb5488

4 files changed

+31
-16
lines changed

t/t5801-remote-helpers.sh

+4-8
Original file line numberDiff line numberDiff line change
@@ -242,13 +242,6 @@ clean_mark () {
242242
sort >$(basename "$1")
243243
}
244244

245-
cmp_marks () {
246-
test_when_finished "rm -rf git.marks testgit.marks" &&
247-
clean_mark ".git/testgit/$1/git.marks" &&
248-
clean_mark ".git/testgit/$1/testgit.marks" &&
249-
test_cmp git.marks testgit.marks
250-
}
251-
252245
test_expect_success 'proper failure checks for fetching' '
253246
(cd local &&
254247
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' '
258251
'
259252

260253
test_expect_success 'proper failure checks for pushing' '
254+
test_when_finished "rm -rf local/git.marks local/testgit.marks" &&
261255
(cd local &&
262256
git checkout -b crash master &&
263257
echo crash >>file &&
264258
git commit -a -m crash &&
265259
test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all &&
266-
cmp_marks origin
260+
clean_mark ".git/testgit/origin/git.marks" &&
261+
clean_mark ".git/testgit/origin/testgit.marks" &&
262+
test_cmp git.marks testgit.marks
267263
)
268264
'
269265

t/t7610-mergetool.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,9 @@ test_expect_success 'mergetool skips autoresolved' '
174174
'
175175

176176
test_expect_success 'mergetool merges all from subdir' '
177+
test_config rerere.enabled false &&
177178
(
178179
cd subdir &&
179-
test_config rerere.enabled false &&
180180
test_must_fail git merge master &&
181181
( yes "r" | git mergetool ../submod ) &&
182182
( yes "d" "d" | git mergetool --no-prompt ) &&

t/t7800-difftool.sh

+4-4
Original file line numberDiff line numberDiff line change
@@ -492,12 +492,12 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' '
492492

493493
test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
494494
git submodule add ./. submod/ule &&
495+
test_config -C submod/ule diff.tool checktrees &&
496+
test_config -C submod/ule difftool.checktrees.cmd '\''
497+
test -d "$LOCAL" && test -d "$REMOTE" && echo good
498+
'\'' &&
495499
(
496500
cd submod/ule &&
497-
test_config diff.tool checktrees &&
498-
test_config difftool.checktrees.cmd '\''
499-
test -d "$LOCAL" && test -d "$REMOTE" && echo good
500-
'\'' &&
501501
echo good >expect &&
502502
git difftool --tool=checktrees --dir-diff HEAD~ >actual &&
503503
test_cmp expect actual

t/test-lib-functions.sh

+22-3
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,14 @@ test_chmod () {
201201

202202
# Unset a configuration variable, but don't fail if it doesn't exist.
203203
test_unconfig () {
204-
git config --unset-all "$@"
204+
config_dir=
205+
if test "$1" = -C
206+
then
207+
shift
208+
config_dir=$1
209+
shift
210+
fi
211+
git ${config_dir:+-C "$config_dir"} config --unset-all "$@"
205212
config_status=$?
206213
case "$config_status" in
207214
5) # ok, nothing to unset
@@ -213,8 +220,15 @@ test_unconfig () {
213220

214221
# Set git config, automatically unsetting it after the test is over.
215222
test_config () {
216-
test_when_finished "test_unconfig '$1'" &&
217-
git config "$@"
223+
config_dir=
224+
if test "$1" = -C
225+
then
226+
shift
227+
config_dir=$1
228+
shift
229+
fi
230+
test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$1'" &&
231+
git ${config_dir:+-C "$config_dir"} config "$@"
218232
}
219233

220234
test_config_global () {
@@ -722,6 +736,11 @@ test_seq () {
722736
# what went wrong.
723737

724738
test_when_finished () {
739+
# We cannot detect when we are in a subshell in general, but by
740+
# doing so on Bash is better than nothing (the test will
741+
# silently pass on other shells).
742+
test "${BASH_SUBSHELL-0}" = 0 ||
743+
error "bug in test script: test_when_finished does nothing in a subshell"
725744
test_cleanup="{ $*
726745
} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
727746
}

0 commit comments

Comments
 (0)