Skip to content

Commit 1b66844

Browse files
pks-tgitster
authored andcommitted
t: prepare test_when_finished ()/test_atexit() for set -e
Both `test_when_finished ()` and `test_atexit ()` build up a chain of cleanup commands by prepending each new command to the existing cleanup string. To preserve the exit code of the test body across cleanup execution, we append the following logic: } && (exit "$eval_ret"); eval_ret=$?; ... The intent of this is to run the cleanup block and then unconditionally restore `eval_ret`. The original behaviour of this is is: +------------------+---------+------------------------------------+ |test body │ cleanup │ old behaviour │ +------------------+---------+------------------------------------+ │pass (eval_ret=0) | pass │ && taken -> (exit 0) -> eval_ret=0 | +------------------+---------+------------------------------------+ │pass (eval_ret=0) | fail │ && not taken -> eval_ret=$? | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | pass │ && taken -> (exit 1) -> eval_ret=1 | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | fail | && not taken -> eval_ret=$? | +------------------+---------+------------------------------------+ This logic will start to fail once we enable `set -e`. When `$eval_ret` is non-zero, the subshell we create will fail, and with `set -e` we'll thus bail out without evaluating the logic after the semicolon. Fix this issue by instead using `|| eval_ret=\$?; ...`. Besides being a bit simpler, it also retains the original behaviour: +------------------+---------+------------------------------------+ |test body │ cleanup │ old behaviour │ +------------------+---------+------------------------------------+ │pass (eval_ret=0) | pass │ || not taken -> eval_ret unchanged | +------------------+---------+------------------------------------+ │pass (eval_ret=0) | fail │ || taken -> eval_ret=$? | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | pass │ || not taken -> eval_ret unchanged | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | fail | || taken -> eval_ret=$? | +------------------+---------+------------------------------------+ Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent bb9e465 commit 1b66844

1 file changed

Lines changed: 2 additions & 2 deletions

File tree

t/test-lib-functions.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,7 @@ test_when_finished () {
15161516
test "${BASH_SUBSHELL-0}" = 0 ||
15171517
BUG "test_when_finished does nothing in a subshell"
15181518
test_cleanup="{ $*
1519-
} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
1519+
} || eval_ret=\$?; $test_cleanup"
15201520
}
15211521

15221522
# This function can be used to schedule some commands to be run
@@ -1544,7 +1544,7 @@ test_atexit () {
15441544
test "${BASH_SUBSHELL-0}" = 0 ||
15451545
BUG "test_atexit does nothing in a subshell"
15461546
test_atexit_cleanup="{ $*
1547-
} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_atexit_cleanup"
1547+
} || eval_ret=\$?; $test_atexit_cleanup"
15481548
}
15491549

15501550
# Deprecated wrapper for "git init", use "git init" directly instead

0 commit comments

Comments
 (0)