Skip to content

Commit cc48ddd

Browse files
peffgitster
authored andcommitted
tests: skip test_eval_ in internal chain-lint
To check for broken &&-chains, we run "fail_117 && $1" as a test snippet, and check the exit code. We use test_eval_ to do so, because that's the way we run the actual test. But we don't need any of its niceties, like "set -x" tracing. In fact, they hinder us, because we have to explicitly disable them. So let's skip that and use "eval" more directly, which is simpler. I had hoped it would also be faster, but it doesn't seem to produce a measurable improvement (probably because it's just running internal shell commands, with no subshells or forks). Note that there is one gotcha: even though we don't intend to run any of the commands if the &&-chain is intact, an error like this: test_expect_success 'broken' ' # this next line breaks the &&-chain true # and then this one is executed even by the linter return 1 ' means we'll "return 1" from the eval, and thus from test_run_(). We actually do notice this in test_expect_success, but only by saying "hey, this test didn't say it was OK, so it must have failed", which is not right (it should say "broken &&-chain"). We can handle this by calling test_eval_inner_() instead, which is our trick for wrapping "return" in a test snippet. But to do that, we have to push the trace code out of that inner function and into test_eval_(). This is arguably where it belonged in the first place, but it never mattered because the "inner_" function had only one caller. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 750b260 commit cc48ddd

File tree

1 file changed

+6
-11
lines changed

1 file changed

+6
-11
lines changed

t/test-lib.sh

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,10 +1041,7 @@ want_trace () {
10411041
# (and we want to make sure we run any cleanup like
10421042
# "set +x").
10431043
test_eval_inner_ () {
1044-
# Do not add anything extra (including LF) after '$*'
1045-
eval "
1046-
want_trace && trace_level_=$(($trace_level_+1)) && set -x
1047-
$*"
1044+
eval "$*"
10481045
}
10491046

10501047
test_eval_ () {
@@ -1069,7 +1066,10 @@ test_eval_ () {
10691066
# be _inside_ the block to avoid polluting the "set -x" output
10701067
#
10711068

1072-
test_eval_inner_ "$@" </dev/null >&3 2>&4
1069+
# Do not add anything extra (including LF) after '$*'
1070+
test_eval_inner_ </dev/null >&3 2>&4 "
1071+
want_trace && trace_level_=$(($trace_level_+1)) && set -x
1072+
$*"
10731073
{
10741074
test_eval_ret_=$?
10751075
if want_trace
@@ -1095,18 +1095,13 @@ test_run_ () {
10951095
expecting_failure=$2
10961096

10971097
if test "${GIT_TEST_CHAIN_LINT:-1}" != 0; then
1098-
# turn off tracing for this test-eval, as it simply creates
1099-
# confusing noise in the "-x" output
1100-
trace_tmp=$trace
1101-
trace=
11021098
# 117 is magic because it is unlikely to match the exit
11031099
# code of other programs
1104-
test_eval_ "fail_117 && $1"
1100+
test_eval_inner_ "fail_117 && $1" </dev/null >&3 2>&4
11051101
if test $? != 117
11061102
then
11071103
BUG "broken &&-chain: $1"
11081104
fi
1109-
trace=$trace_tmp
11101105
fi
11111106

11121107
setup_malloc_check

0 commit comments

Comments
 (0)