From ac6e12f9b7794999ba5c9db87b01215ef76e5f33 Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Tue, 18 Sep 2018 23:29:34 +0000 Subject: [PATCH 1/6] t/README: correct spelling of "uncommon" Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE. Signed-off-by: Ben Peart Signed-off-by: Junio C Hamano --- t/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/README b/t/README index 9028b47d92..56a417439c 100644 --- a/t/README +++ b/t/README @@ -315,7 +315,7 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than bytes. -GIT_TEST_OE_DELTA_SIZE= exercises the uncomon pack-objects code +GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. From 5aacc63f1e8414a12105faa7647c4c11f8acd02d Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Tue, 18 Sep 2018 23:29:34 +0000 Subject: [PATCH 2/6] preload-index: use git_env_bool() not getenv() for customization GIT_FORCE_PRELOAD_TEST is only checked for presence by using getenv(). Use git_env_bool() instead so that GIT_FORCE_PRELOAD_TEST=false can work as expected. Signed-off-by: Ben Peart Signed-off-by: Junio C Hamano --- preload-index.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/preload-index.c b/preload-index.c index 71cd2437a3..0a4e2933bb 100644 --- a/preload-index.c +++ b/preload-index.c @@ -5,6 +5,7 @@ #include "pathspec.h" #include "dir.h" #include "fsmonitor.h" +#include "config.h" #ifdef NO_PTHREADS static void preload_index(struct index_state *index, @@ -84,7 +85,7 @@ static void preload_index(struct index_state *index, return; threads = index->cache_nr / THREAD_COST; - if ((index->cache_nr > 1) && (threads < 2) && getenv("GIT_FORCE_PRELOAD_TEST")) + if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_FORCE_PRELOAD_TEST", 0)) threads = 2; if (threads < 2) return; From 4cb54d0aa8e2b8de6f6bafd5c2baad0db5ce9dc7 Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Tue, 18 Sep 2018 23:29:35 +0000 Subject: [PATCH 3/6] fsmonitor: update GIT_TEST_FSMONITOR support Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Remove the outdated instructions on how to run the test suite utilizing fsmonitor now that it is properly documented in t/README. Signed-off-by: Ben Peart Signed-off-by: Junio C Hamano --- config.c | 2 +- t/README | 4 ++++ t/t1700-split-index.sh | 2 +- t/t7519-status-fsmonitor.sh | 7 ------- t/test-lib.sh | 20 ++++++++++++++++++++ 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 56a417439c..47165f7eab 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests ------------ diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index b3b4d83eaf..f6a856f24c 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -6,7 +6,7 @@ test_description='split index mode tests' # We need total control of index splitting here sane_unset GIT_TEST_SPLIT_INDEX -sane_unset GIT_FSMONITOR_TEST +sane_unset GIT_TEST_FSMONITOR test_expect_success 'enable split index' ' git config splitIndex.maxPercentChange 100 && diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..06f59822e4 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -4,13 +4,6 @@ test_description='git status with file system watcher' . ./test-lib.sh -# -# To run the entire git test suite using fsmonitor: -# -# copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. -# - # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' # "git update-index --fsmonitor" can be used to get the extension written # before testing the results. diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..653688c067 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -140,6 +140,26 @@ then export GIT_INDEX_VERSION fi +check_var_migration () { + 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" + echo >&2 "hint: set $new_name too during the transition period" + eval "$new_name=\$$old_name" + ;; + isset,isset) + # do this later + # echo >&2 "warning: $old_name is now $new_name" + # echo >&2 "hint: remove $old_name" + ;; + esac +} + +check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null || From 1f357b045b503873626a72ac97797352a3cd2e87 Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Tue, 18 Sep 2018 23:29:36 +0000 Subject: [PATCH 4/6] read-cache: update TEST_GIT_INDEX_VERSION support Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart Signed-off-by: Junio C Hamano --- Makefile | 6 +++--- t/README | 4 ++++ t/test-lib.sh | 14 ++++++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/t/README b/t/README index 47165f7eab..9b13f6d12e 100644 --- a/t/README +++ b/t/README @@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor code path for utilizing a file system monitor to speed up detecting new or changed files. +GIT_TEST_INDEX_VERSION= exercises the index read/write code path +for the index version specified. Can be set to any valid version +(currently 2, 3, or 4). + Naming Tests ------------ diff --git a/t/test-lib.sh b/t/test-lib.sh index 653688c067..e80c84d13c 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,12 +134,6 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" -then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" - export GIT_INDEX_VERSION -fi - check_var_migration () { old_name=$1 new_name=$2 eval "old_isset=\${${old_name}:+isset}" @@ -159,6 +153,14 @@ check_var_migration () { } check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION + +# Use specific version of the index file format +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" +then + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" + export GIT_INDEX_VERSION +fi # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind From 5765d97b71d7489f927354e1704fc0d1d7ef90a0 Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Tue, 18 Sep 2018 23:29:37 +0000 Subject: [PATCH 5/6] preload-index: update GIT_FORCE_PRELOAD_TEST support Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart Signed-off-by: Junio C Hamano --- preload-index.c | 2 +- t/README | 3 +++ t/t7519-status-fsmonitor.sh | 4 ++-- t/test-lib.sh | 1 + 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/preload-index.c b/preload-index.c index 0a4e2933bb..a850e197c2 100644 --- a/preload-index.c +++ b/preload-index.c @@ -85,7 +85,7 @@ static void preload_index(struct index_state *index, return; threads = index->cache_nr / THREAD_COST; - if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_FORCE_PRELOAD_TEST", 0)) + if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_TEST_PRELOAD_INDEX", 0)) threads = 2; if (threads < 2) return; diff --git a/t/README b/t/README index 9b13f6d12e..5670c7aad0 100644 --- a/t/README +++ b/t/README @@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write code path for the index version specified. Can be set to any valid version (currently 2, 3, or 4). +GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path +by overriding the minimum number of cache entries required per thread. + Naming Tests ------------ diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 06f59822e4..3f0dd98010 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -238,9 +238,9 @@ do git config core.preloadIndex $preload_val && if test $preload_val = true then - GIT_FORCE_PRELOAD_TEST=$preload_val; export GIT_FORCE_PRELOAD_TEST + GIT_TEST_PRELOAD_INDEX=$preload_val; export GIT_TEST_PRELOAD_INDEX else - unset GIT_FORCE_PRELOAD_TEST + sane_unset GIT_TEST_PRELOAD_INDEX fi ' diff --git a/t/test-lib.sh b/t/test-lib.sh index e80c84d13c..8ef86e05a3 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -154,6 +154,7 @@ check_var_migration () { check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION +check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX # Use specific version of the index file format if test -n "${GIT_TEST_INDEX_VERSION:+isset}" From 4231d1ba995379974401062349c3281d7a821be5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 20 Sep 2018 11:43:43 -0700 Subject: [PATCH 6/6] 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 --- 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"