Re: [PATCH 01/25] t/test-lib: introduce --chain-lint option
From: SZEDER Gábor <hidden>
Date: 2016-06-15 23:04:17
Quoting Jeff King [off-list ref]:
However, there are a number of places it cannot reach:
- it cannot find a failure to break out of loops on error,
like:
cmd1 &&
for i in a b c; do
cmd2 $i
done &&
cmd3
which will not notice failures of "cmd2 a" or "cmd b"s/cmd b/cmd2 b/ ?
- it cannot find a missing &&-chain inside a block or
subfunction, like:
foo () {
cmd1
cmd2
}
foo &&
bar
which will not notice a failure of cmd1.
And inside subshells. I think it's worth mentioning, too, because
subshells are used frequently when setting environment variables
( GIT_FOO=bar && export GIT_FOO && cmd1 && ... ) && test_cmp
or changing directory
( cd subdir && cmd1 && ... ) && test_cmp
I was wondering whether we could do better here with helper functions,
something along the lines of:
# Set and export environment variable, automatically unsetting it after
# the test is over
test_setenv () {
eval "$1=\$2" &&
export "$1" &&
# sane_unset, to allow unsetting during the test
test_when_finished "sane_unset '$1'"
}
# Change to given directory, automatically return to current working
# directory after the test is over
test_cd () {
test_when_finished "cd '$PWD'" &&
cd "$1"
}
With these the above examples would become
test_setenv GIT_FOO bar && cmd1 && ... && test_cmp
and
test_cd subdir && cmd1 && ... && test_cmp
which means increased coverage for --chain-lint.
Thanks for working on this. I looked into this after seeing Jonathan's
patch back then, got quite far but never reached a chain-lint-clean
state, and only sent patches for the two most amusing breakages
(ddeaf7ef0d, 056f34bbcd).
I'm glad it's off my TODO list and I don't have to rebase a 1.5 year old
branch to current master :)
Gábor
quoted hunk ↗ jump to hunk
- it only checks tests that you run; every platform will have some tests skipped due to missing prequisites, so it's impossible to say from one run that the test suite is free of broken &&-chains. However, all tests get run by _somebody_, so eventually we will notice problems. - it does not operate on test_when_finished or prerequisite blocks. It could, but these tends to be much shorter and less of a problem, so I punted on them in this patch. This patch was inspired by an earlier patch by Jonathan Nieder: http://article.gmane.org/gmane.comp.version-control.git/235913 This implementation and all bugs are mine. Signed-off-by: Jeff King <redacted> --- t/README | 10 ++++++++++ t/test-lib.sh | 16 ++++++++++++++++ 2 files changed, 26 insertions(+)diff --git a/t/README b/t/README index d5bb0c9..35438bc 100644 --- a/t/README +++ b/t/README@@ -168,6 +168,16 @@ appropriately before running "make". Using this option with a RAM-based filesystem (such as tmpfs) can massively speed up the test suite. +--chain-lint:: +--no-chain-lint:: + If --chain-lint is enabled, the test harness will check each + test to make sure that it properly "&&-chains" all commands (so + that a failure in the middle does not go unnoticed by the final + exit code of the test). This check is performed in addition to + running the tests themselves. You may also enable or disable + this feature by setting the GIT_TEST_CHAIN_LINT environment + variable to "1" or "0", respectively. + You can also set the GIT_TEST_INSTALLED environment variable to the bindir of an existing git installation to test that installation. You still need to have built this git sandbox, from which variousdiff --git a/t/test-lib.sh b/t/test-lib.sh index c096778..50b3d3f 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh@@ -232,6 +232,12 @@ do --root=*) root=$(expr "z$1" : 'z[^=]*=\(.*\)') shift ;; + --chain-lint) + GIT_TEST_CHAIN_LINT=1 + shift ;; + --no-chain-lint) + GIT_TEST_CHAIN_LINT=0 + shift ;; -x) trace=t verbose=t@@ -524,6 +530,16 @@ test_eval_ () { test_run_ () { test_cleanup=: expecting_failure=$2 + + if test "${GIT_TEST_CHAIN_LINT:-0}" != 0; then + # 117 is magic because it is unlikely to match the exit + # code of other programs + test_eval_ "(exit 117) && $1" + if test "$?" != 117; then + error "bug in the test script: broken &&-chain: $1" + fi + fi + setup_malloc_check test_eval_ "$1" eval_ret=$? --2.3.3.520.g3cfbb5d