From: Junio C Hamano <gitster@pobox.com>
To: Ben Peart <benpeart@microsoft.com>
Cc: "git\@vger.kernel.org" <git@vger.kernel.org>,
"jrnieder\@gmail.com" <jrnieder@gmail.com>,
Ben Peart <Ben.Peart@microsoft.com>
Subject: Re*: [PATCH v3 0/5] Cleanup pass on special test setups
Date: Thu, 20 Sep 2018 11:43:43 -0700 [thread overview]
Message-ID: <xmqqtvmkyppc.fsf_-_@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180918232916.57736-1-benpeart@microsoft.com> (Ben Peart's message of "Tue, 18 Sep 2018 23:29:33 +0000")
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 suite
The 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(+)
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"
next prev parent reply other threads:[~2018-09-20 18:43 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 14:37 [PATCH v1 0/4] Cleanup pass on special test setups Ben Peart
2018-09-14 14:37 ` [PATCH v1 1/4] correct typo/spelling error in t/README Ben Peart
2018-09-14 14:37 ` [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
2018-09-14 16:59 ` Junio C Hamano
2018-09-14 17:03 ` Junio C Hamano
2018-09-14 17:15 ` Junio C Hamano
2018-09-14 18:05 ` Ben Peart
2018-09-14 18:18 ` Junio C Hamano
2018-09-14 20:13 ` [PATCH v2 0/5] Cleanup pass on special test setups Ben Peart
2018-09-14 20:13 ` [PATCH v2 1/5] correct typo/spelling error in t/README Ben Peart
2018-09-14 20:43 ` Jonathan Nieder
2018-09-14 20:14 ` [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean Ben Peart
2018-09-14 20:51 ` Jonathan Nieder
2018-09-14 21:54 ` Junio C Hamano
2018-09-14 20:14 ` [PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
2018-09-14 20:14 ` [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
2018-09-14 22:13 ` Junio C Hamano
2018-09-14 22:26 ` Junio C Hamano
2018-09-14 20:14 ` [PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support Ben Peart
2018-09-14 14:37 ` [PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
2018-09-14 14:37 ` [PATCH v1 4/4] preload-index: update GIT_FORCE_PRELOAD_TEST support Ben Peart
2018-09-18 23:29 ` [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
2018-09-18 23:29 ` [PATCH v3 2/5] preload-index: use git_env_bool() not getenv() for customization Ben Peart
2018-09-18 23:29 ` [PATCH v3 1/5] t/README: correct spelling of "uncommon" Ben Peart
2018-09-18 23:29 ` [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
2018-09-28 10:01 ` SZEDER Gábor
2018-09-28 14:21 ` Ben Peart
2018-09-28 14:27 ` Ben Peart
2018-09-28 18:43 ` Junio C Hamano
2018-09-18 23:29 ` [PATCH v3 4/5] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
2018-09-18 23:29 ` [PATCH v3 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support Ben Peart
2018-09-20 18:43 ` Junio C Hamano [this message]
2018-09-25 18:44 ` Re*: [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqtvmkyppc.fsf_-_@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=Ben.Peart@microsoft.com \
--cc=benpeart@microsoft.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).