Re*: [PATCH v3 0/5] Cleanup pass on special test setups
Ben Peart <benpeart@microsoft.com> writes:
> This round has one code change based on feedback. Other changes are just
> rewording commit messages.
Thanks. I think the only remaining issue is what to do with the interaction between extra/additional error message that comes from the updates in 3/5 and the test framework selftest in t0000.
-- >8 --
Subject: t0000: do not get self-test disrupted by environment warnings
The test framework test-lib.sh itself would want to give warnings and hints, e.g. when it sees a deprecated environment variable is in use that we want to encourage users to migrate to another variable.
The self-test of test framework done in t0000 however do not expect to see these warnings and hints, so depending on the settings of environment variables, a running test may or may not produce these messages to the standard error output, breaking the expectations of self-test test framework does on itself. Here is what we see:
$ TEST_GIT_INDEX_VERSION=4 sh t0000-basic.sh -i -v
...
'err' is not empty, it contains:
warning: TEST_GIT_INDEX_VERSION is now GIT_TEST_INDEX_VERSION
hint: set GIT_TEST_INDEX_VERSION too during the transition period
not ok 5 - pretend we have a fully passing test suiteThe following quick attempt to work it around does not work, because some tests in t0000 do want to see expected errors from the test framework itself.
t/t0000-basic.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 850f651e4e..88c6ed4696 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -88,7 +88,7 @@ _run_sub_test_lib_test_common () {
' # Point to the t/test-lib.sh, which isn't in ../ as usual
- . "\$TEST_DIRECTORY"/test-lib.sh
+ . "\$TEST_DIRECTORY"/test-lib.sh >/dev/null 2>&1
EOF
cat >>"$name.sh" &&
chmod +x "$name.sh" &&There are a few possible ways to work this around:
* We could strip the warning: and hint: unconditionally from the
error output before the error messages are checked in the
self-test (helper functions check_sub_test_lib_test_err and
check_sub_test_lib_test); the problem with this approach is that
it will make it impossible to write self-tests to ensure that
right warnings and hints are given.
* We could force a sane environment settings before the test helper
_run_sub_test_lib_test_common dot-sources test-lib.sh; the
problem with this approach is that _run_sub_test_lib_test_common
now needs to be aware of what pairs of environment variables are
checked in test-lib.sh using check_var_migration helper.
The final patch I came up with is probably the solution that is least bad. Set a variable to tell test-lib.sh that we are running a self-test, so that various pieces in test-lib.sh can react to keep the output stable.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0000-basic.sh | 4 ++++
t/test-lib.sh | 8 ++++++++
2 files changed, 12 insertions(+)
Show changes to 2 files +12 −0
t/t0000-basic.sh, t/test-lib.sh
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 850f651e4e..52c02b7c7e 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -87,6 +87,10 @@ _run_sub_test_lib_test_common () {
passing metrics
'
+ # Tell the framework that we are self-testing to make sure
+ # it yields a stable result.
+ GIT_TEST_FRAMEWORK_SELFTEST=t &&
+
# Point to the t/test-lib.sh, which isn't in ../ as usual
. "\$TEST_DIRECTORY"/test-lib.sh
EOF
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8ef86e05a3..364a11ea25 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -135,9 +135,17 @@ GIT_TRACE_BARE=1
export GIT_TRACE_BARE
check_var_migration () {
+ # the warnings and hints given from this helper depends
+ # on end-user settings, which will disrupt the self-test
+ # done on the test framework itself.
+ case "$GIT_TEST_FRAMEWORK_SELFTEST" in
+ t) return ;;
+ esac
+
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
eval "new_isset=\${${new_name}:+isset}"
+
case "$old_isset,$new_isset" in
isset,)
echo >&2 "warning: $old_name is now $new_name"
Ben Peart
