git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH v1 0/4] Cleanup pass on special test setups
@ 2018-09-14 14:37 Ben Peart
  2018-09-14 14:37 ` [PATCH v1 1/4] correct typo/spelling error in t/README Ben Peart
                   ` (4 more replies)
  0 siblings, 5 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 14:37 UTC (permalink / raw)
  To: git; +Cc: gitster, t.gummerer, avarab, Ben Peart

As documented in t/README, the whole test suite can be run to test some
special features that cannot be easily covered by a few specific test cases.

Not all of these that exist in the code have been named consistantly and
documented in r/README which leads to a discoverability problem.  Update
several of these variables to follow the same naming pattern and document
them properly.

To facilitate transitioning from the old names to the new names, add logic
in t/test-lib.sh to give an error when the old variable is set to let people
know they need to update their environment to use the new variable. If the
new variable is also set, just give a warning so they can eventually remove
the old variable.

Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/eff73d737e
Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v1 && git checkout eff73d737e

Ben Peart (4):
  correct typo/spelling error in t/README
  fsmonitor: update GIT_TEST_FSMONITOR support
  read-cache: update TEST_GIT_INDEX_VERSION support
  preload-index: update GIT_FORCE_PRELOAD_TEST support

 Makefile                    |  6 +++---
 config.c                    |  2 +-
 preload-index.c             |  3 ++-
 t/README                    | 13 ++++++++++++-
 t/t1700-split-index.sh      |  2 +-
 t/t7519-status-fsmonitor.sh |  6 +++---
 t/test-lib.sh               | 37 +++++++++++++++++++++++++++++++++++--
 7 files changed, 57 insertions(+), 12 deletions(-)


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
-- 
2.18.0.windows.1



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 1/4] correct typo/spelling error in t/README
  2018-09-14 14:37 [PATCH v1 0/4] Cleanup pass on special test setups Ben Peart
@ 2018-09-14 14:37 ` Ben Peart
  2018-09-14 14:37 ` [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 14:37 UTC (permalink / raw)
  To: git; +Cc: gitster, t.gummerer, avarab, Ben Peart, Ben Peart

Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 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
 <n> bytes.
 
-GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code
+GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
  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 ` Ben Peart
  2018-09-14 16:59   ` Junio C Hamano
  2018-09-14 17:15   ` Junio C Hamano
  2018-09-14 14:37 ` [PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 14:37 UTC (permalink / raw)
  To: git; +Cc: gitster, t.gummerer, avarab, Ben Peart, Ben Peart

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 an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 config.c                    |  2 +-
 t/README                    |  4 ++++
 t/t1700-split-index.sh      |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh               | 11 +++++++++++
 5 files changed, 18 insertions(+), 3 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=<n> 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..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # 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.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..0ef111d808 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,17 @@ then
 	export GIT_INDEX_VERSION
 fi
 
+if test -n "$GIT_FSMONITOR_TEST"
+then
+	if test -n "$GIT_TEST_FSMONITOR"
+	then
+		echo "warning: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
+	else
+		echo "error: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
+		exit 1
+	fi
+fi
+
 # 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 ||
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support
  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 14:37 ` 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
  4 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 14:37 UTC (permalink / raw)
  To: git; +Cc: gitster, t.gummerer, avarab, Ben Peart, Ben Peart

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 an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 Makefile      |  6 +++---
 t/README      |  4 ++++
 t/test-lib.sh | 15 +++++++++++++--
 3 files changed, 20 insertions(+), 5 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=<n> 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 0ef111d808..5f5f0f4b55 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,12 +134,23 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-	GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+	GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
 	export GIT_INDEX_VERSION
 fi
 
+if test -n "$TEST_GIT_INDEX_VERSION"
+then
+	if test -n "$GIT_TEST_INDEX_VERSION"
+	then
+		echo "warning: the TEST_GIT_INDEX_VERSION variable has been renamed to GIT_TEST_INDEX_VERSION"
+	else
+		echo "error: the TEST_GIT_INDEX_VERSION variable has been renamed to GIT_TEST_INDEX_VERSION"
+		exit 1
+	fi
+fi
+
 if test -n "$GIT_FSMONITOR_TEST"
 then
 	if test -n "$GIT_TEST_FSMONITOR"
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 4/4] preload-index: update GIT_FORCE_PRELOAD_TEST support
  2018-09-14 14:37 [PATCH v1 0/4] Cleanup pass on special test setups Ben Peart
                   ` (2 preceding siblings ...)
  2018-09-14 14:37 ` [PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
@ 2018-09-14 14:37 ` Ben Peart
  2018-09-18 23:29 ` [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
  4 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 14:37 UTC (permalink / raw)
  To: git; +Cc: gitster, t.gummerer, avarab, Ben Peart, Ben Peart

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 an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 preload-index.c             |  3 ++-
 t/README                    |  3 +++
 t/t7519-status-fsmonitor.sh |  4 ++--
 t/test-lib.sh               | 11 +++++++++++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/preload-index.c b/preload-index.c
index 71cd2437a3..a850e197c2 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_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=<n> 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=<boolean> 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 d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,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 5f5f0f4b55..3f447b8ddc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -162,6 +162,17 @@ then
 	fi
 fi
 
+if test -n "$GIT_FORCE_PRELOAD_TEST"
+then
+	if test -n "$GIT_TEST_PRELOAD_INDEX"
+	then
+		echo "warning: the GIT_FORCE_PRELOAD_TEST variable has been renamed to GIT_TEST_PRELOAD_INDEX"
+	else
+		echo "error: the GIT_FORCE_PRELOAD_TEST variable has been renamed to GIT_TEST_PRELOAD_INDEX"
+		exit 1
+	fi
+fi
+
 # 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 ||
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
  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
  1 sibling, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 16:59 UTC (permalink / raw)
  To: Ben Peart; +Cc: git\, t.gummerer\, avarab\, Ben Peart

Ben Peart <benpeart@microsoft.com> writes:

> +if test -n "$GIT_FSMONITOR_TEST"
> +then
> +	if test -n "$GIT_TEST_FSMONITOR"
> +	then
> +		echo "warning: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
> +	else
> +		echo "error: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
> +		exit 1
> +	fi
> +fi

I would have expected that, because we are now doing multiple pairs
of variables in a single series, we would add a helper function that
can be called like so:

	check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR

in the earliest step.  Perhaps something like this.

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 "error: $old_name is now $new_name"
		exit 1 ;;
	isset,isset)
		# enable this, once $old_name no longer is valid anywhere
		# echo >&2 "warning: $old_name is now $new_name"
		# echo >&2 "hint: remove $old_name"
		;;
	esac
}


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
  2018-09-14 16:59   ` Junio C Hamano
@ 2018-09-14 17:03     ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 17:03 UTC (permalink / raw)
  To: Ben Peart; +Cc: git\, t.gummerer\, avarab\, Ben Peart

Junio C Hamano <gitster@pobox.com> writes:

> Ben Peart <benpeart@microsoft.com> writes:
>
>> +if test -n "$GIT_FSMONITOR_TEST"
>> +then
>> +	if test -n "$GIT_TEST_FSMONITOR"
>> +	then
>> +		echo "warning: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
>> +	else
>> +		echo "error: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
>> +		exit 1
>> +	fi
>> +fi
>
> I would have expected that, because we are now doing multiple pairs
> of variables in a single series, we would add a helper function that
> can be called like so:
>
> 	check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>
> in the earliest step.  Perhaps something like this.
>
> 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 "error: $old_name is now $new_name"
> 		exit 1 ;;
> 	isset,isset)
> 		# enable this, once $old_name no longer is valid anywhere
> 		# echo >&2 "warning: $old_name is now $new_name"
> 		# echo >&2 "hint: remove $old_name"
> 		;;
> 	esac
> }

Alternatively, we could do this, to warn and then migrate the value
given to the old variable automatically to the new variable and let
the test proceed.

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
}

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
  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:15   ` Junio C Hamano
  2018-09-14 18:05     ` Ben Peart
  1 sibling, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 17:15 UTC (permalink / raw)
  To: Ben Peart; +Cc: git\, t.gummerer\, avarab\, Ben Peart

Ben Peart <benpeart@microsoft.com> writes:

> 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");

Sorry for not noticing earlier, but unlike 4/4 that changed
getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
getenv(VAR), meaning "if it is set to any non-empty string, it is
true".  Is there a reason for this discrepancy?

I _think_ the renaming should be done without getting mixed with
other changes like the git_env_bool() done in 4/4.  The idea to use
git_env_bool() in stead of getenv() may be a good one, but then we
should consistently do so when appropriate, and that would make a
fine theme for another topic.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
  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
  0 siblings, 2 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 18:05 UTC (permalink / raw)
  To: Junio C Hamano, Ben Peart; +Cc: git, t.gummerer, avarab, Ben Peart



On 9/14/2018 1:15 PM, Junio C Hamano wrote:
> Ben Peart <benpeart@microsoft.com> writes:
> 
>> 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");
> 
> Sorry for not noticing earlier, but unlike 4/4 that changed
> getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
> getenv(VAR), meaning "if it is set to any non-empty string, it is
> true".  Is there a reason for this discrepancy?
> 

The difference here is that core.fsmonitor isn't a boolean value.  It is 
a string to a command that is executed so it can't be moved over to 
get_env_bool().

> I _think_ the renaming should be done without getting mixed with
> other changes like the git_env_bool() done in 4/4.  The idea to use
> git_env_bool() in stead of getenv() may be a good one, but then we
> should consistently do so when appropriate, and that would make a
> fine theme for another topic.
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
  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
  1 sibling, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 18:18 UTC (permalink / raw)
  To: Ben Peart; +Cc: Ben Peart, git\, t.gummerer\, avarab\, Ben Peart

Ben Peart <peartben@gmail.com> writes:

> The difference here is that core.fsmonitor isn't a boolean value.  It
> is a string to a command that is executed so it can't be moved over to
> get_env_bool().

Ah, of course ;-)

Then please take the following as a review comment for 4/4; checking
if each getenv(VAR) should or should not become git_env_bool() and
updating them should be done as a separate change for variables
whether they are being renamed or not in this series.

>> I _think_ the renaming should be done without getting mixed with
>> other changes like the git_env_bool() done in 4/4.  The idea to use
>> git_env_bool() in stead of getenv() may be a good one, but then we
>> should consistently do so when appropriate, and that would make a
>> fine theme for another topic.
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 0/5] Cleanup pass on special test setups
  2018-09-14 18:05     ` Ben Peart
  2018-09-14 18:18       ` Junio C Hamano
@ 2018-09-14 20:13       ` Ben Peart
  2018-09-14 20:13         ` [PATCH v2 1/5] correct typo/spelling error in t/README Ben Peart
                           ` (4 more replies)
  1 sibling, 5 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 20:13 UTC (permalink / raw)
  To: peartben; +Cc: Ben Peart, avarab, Ben Peart, git, gitster, t.gummerer

Changes this round are to use Junio's more elegant script to test and warn
about using old variables and munging which changes are in which commit.

Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/79d62d39e4
Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v2 && git checkout 79d62d39e4


### Interdiff (v1..v2):

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 3f447b8ddc..17a56f44ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,38 +140,27 @@ then
 	export GIT_INDEX_VERSION
 fi
 
-if test -n "$TEST_GIT_INDEX_VERSION"
-then
-	if test -n "$GIT_TEST_INDEX_VERSION"
-	then
-		echo "warning: the TEST_GIT_INDEX_VERSION variable has been renamed to GIT_TEST_INDEX_VERSION"
-	else
-		echo "error: the TEST_GIT_INDEX_VERSION variable has been renamed to GIT_TEST_INDEX_VERSION"
-		exit 1
-	fi
-fi
-
-if test -n "$GIT_FSMONITOR_TEST"
-then
-	if test -n "$GIT_TEST_FSMONITOR"
-	then
-		echo "warning: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
-	else
-		echo "error: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
-		exit 1
-	fi
-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
+}
 
-if test -n "$GIT_FORCE_PRELOAD_TEST"
-then
-	if test -n "$GIT_TEST_PRELOAD_INDEX"
-	then
-		echo "warning: the GIT_FORCE_PRELOAD_TEST variable has been renamed to GIT_TEST_PRELOAD_INDEX"
-	else
-		echo "error: the GIT_FORCE_PRELOAD_TEST variable has been renamed to GIT_TEST_PRELOAD_INDEX"
-		exit 1
-	fi
-fi
+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
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind


### Patches

Ben Peart (5):
  correct typo/spelling error in t/README
  preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
  fsmonitor: update GIT_TEST_FSMONITOR support
  read-cache: update TEST_GIT_INDEX_VERSION support
  preload-index: update GIT_FORCE_PRELOAD_TEST support

 Makefile                    |  6 +++---
 config.c                    |  2 +-
 preload-index.c             |  3 ++-
 t/README                    | 13 ++++++++++++-
 t/t1700-split-index.sh      |  2 +-
 t/t7519-status-fsmonitor.sh |  6 +++---
 t/test-lib.sh               | 26 ++++++++++++++++++++++++--
 7 files changed, 46 insertions(+), 12 deletions(-)


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
-- 
2.18.0.windows.1



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 1/5] correct typo/spelling error in t/README
  2018-09-14 20:13       ` [PATCH v2 0/5] Cleanup pass on special test setups Ben Peart
@ 2018-09-14 20:13         ` 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
                           ` (3 subsequent siblings)
  4 siblings, 1 reply; 28+ messages in thread
From: Ben Peart @ 2018-09-14 20:13 UTC (permalink / raw)
  To: peartben; +Cc: Ben Peart, avarab, Ben Peart, git, gitster, t.gummerer

Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 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
 <n> bytes.
 
-GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code
+GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
  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:14         ` Ben Peart
  2018-09-14 20:51           ` Jonathan Nieder
  2018-09-14 20:14         ` [PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
                           ` (2 subsequent siblings)
  4 siblings, 1 reply; 28+ messages in thread
From: Ben Peart @ 2018-09-14 20:14 UTC (permalink / raw)
  To: peartben; +Cc: Ben Peart, avarab, Ben Peart, git, gitster, t.gummerer

Teach GIT_FORCE_PRELOAD_TEST to take a boolean to turn on or off this test
feature instead of simply testing for existance.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 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;
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
  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:14         ` [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean Ben Peart
@ 2018-09-14 20:14         ` Ben Peart
  2018-09-14 20:14         ` [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
  2018-09-14 20:14         ` [PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support Ben Peart
  4 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 20:14 UTC (permalink / raw)
  To: peartben; +Cc: Ben Peart, avarab, Ben Peart, git, gitster, t.gummerer

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.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 config.c                    |  2 +-
 t/README                    |  4 ++++
 t/t1700-split-index.sh      |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh               | 20 ++++++++++++++++++++
 5 files changed, 27 insertions(+), 3 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=<n> 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..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # 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.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
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 ||
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
  2018-09-14 20:13       ` [PATCH v2 0/5] Cleanup pass on special test setups Ben Peart
                           ` (2 preceding siblings ...)
  2018-09-14 20:14         ` [PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
@ 2018-09-14 20:14         ` Ben Peart
  2018-09-14 22:13           ` Junio C Hamano
  2018-09-14 20:14         ` [PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support Ben Peart
  4 siblings, 1 reply; 28+ messages in thread
From: Ben Peart @ 2018-09-14 20:14 UTC (permalink / raw)
  To: peartben; +Cc: Ben Peart, avarab, Ben Peart, git, gitster, t.gummerer

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 <Ben.Peart@microsoft.com>
---
 Makefile      | 6 +++---
 t/README      | 4 ++++
 t/test-lib.sh | 5 +++--
 3 files changed, 10 insertions(+), 5 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=<n> 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..397eb71578 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,9 +134,9 @@ export EDITOR
 GIT_TRACE_BARE=1
 export GIT_TRACE_BARE
 
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
 then
-	GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+	GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
 	export GIT_INDEX_VERSION
 fi
 
@@ -159,6 +159,7 @@ check_var_migration () {
 }
 
 check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support
  2018-09-14 20:13       ` [PATCH v2 0/5] Cleanup pass on special test setups Ben Peart
                           ` (3 preceding siblings ...)
  2018-09-14 20:14         ` [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
@ 2018-09-14 20:14         ` Ben Peart
  4 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-14 20:14 UTC (permalink / raw)
  To: peartben; +Cc: Ben Peart, avarab, Ben Peart, git, gitster, t.gummerer

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 <Ben.Peart@microsoft.com>
---
 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=<n> 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=<boolean> 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 d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,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 397eb71578..17a56f44ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -160,6 +160,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
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/5] correct typo/spelling error in t/README
  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
  0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Nieder @ 2018-09-14 20:43 UTC (permalink / raw)
  To: Ben Peart; +Cc: peartben, Ben Peart, avarab, git, gitster, t.gummerer

Hi,

Ben Peart wrote:

> Subject: correct typo/spelling error in t/README

nit: what is the difference between a typo/spelling error and another
kind of spelling error?  Maybe this could be something like

	t/README: correct spelling of "uncommon"

which makes it crystal clear what the patch will do.

> Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE

The commit message should consist of complete sentences, so this is
missing a period.  Alternatively, I think it would be fine to omit the
sentence altogether.

> Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
> ---
>  t/README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This appears to be the only usage of uncomon in the code base.  Thanks
for fixing it.

With or without the commit message tweaks mentioned above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
  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
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Nieder @ 2018-09-14 20:51 UTC (permalink / raw)
  To: Ben Peart; +Cc: peartben, Ben Peart, avarab, git, gitster, t.gummerer

Ben Peart wrote:

> Subject: preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean

Reading this subject line alone (e.g. in "git log --oneline" output),
it's not obvious to me what this patch will do.

What behavior change does it make / what will it make newly possible?

Maybe something like:

	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.

> Teach GIT_FORCE_PRELOAD_TEST to take a boolean to turn on or off this test
> feature instead of simply testing for existance.
>
> Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
> ---
>  preload-index.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Can you say a little about how this came up?  Was it just noticed
while reading the code, or did it come up in practice?

I wonder if a more useful knob would be a GIT_FORCE_PRELOAD_THREADS
setting, but that's orthogonal to this change.

Thanks and hope that helps,
Jonathan

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
  2018-09-14 20:51           ` Jonathan Nieder
@ 2018-09-14 21:54             ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 21:54 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Ben Peart, peartben\, Ben Peart, avarab\, git\, t.gummerer\

Jonathan Nieder <jrnieder@gmail.com> writes:

> Maybe something like:
>
> 	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.

That is much better description.  Also

	$ cd t && GIT_FORCE_PRELOAD_TEST=t ./t0000-basic.sh

would have allowed us to enable the feature in the older world, but
I suspect it would instead fail the test, saying 't is not a bool
nor int'.

So strictly speaking, it is a backward incompatible change.  I am
not sure if I like it.

>> Teach GIT_FORCE_PRELOAD_TEST to take a boolean to turn on or off this test
>> feature instead of simply testing for existance.

s/existance/existence/?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
  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
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 22:13 UTC (permalink / raw)
  To: Ben Peart; +Cc: peartben\, Ben Peart, avarab\, git\, t.gummerer\

Ben Peart <benpeart@microsoft.com> writes:

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 653688c067..397eb71578 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -134,9 +134,9 @@ export EDITOR
>  GIT_TRACE_BARE=1
>  export GIT_TRACE_BARE
>  
> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
>  then
> -	GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
> +	GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
>  	export GIT_INDEX_VERSION
>  fi

Is this done a bit before ...

> @@ -159,6 +159,7 @@ check_var_migration () {
>  }
>  
>  check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION

... this has a chance to kick in to say things like "Whoa you have
TEST_GIT_INDEX_VERSION that is an old spelling of
GIT_TEST_INDEX_VERSION", isn't it?

>  # Add libc MALLOC and MALLOC_PERTURB test
>  # only if we are not executing the test with valgrind

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
  2018-09-14 22:13           ` Junio C Hamano
@ 2018-09-14 22:26             ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2018-09-14 22:26 UTC (permalink / raw)
  To: Ben Peart; +Cc: peartben\, Ben Peart, avarab\, git\, t.gummerer\

Junio C Hamano <gitster@pobox.com> writes:

> Ben Peart <benpeart@microsoft.com> writes:
>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 653688c067..397eb71578 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -134,9 +134,9 @@ export EDITOR
>>  GIT_TRACE_BARE=1
>>  export GIT_TRACE_BARE
>>  
>> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
>> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
>>  then
>> -	GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
>> +	GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
>>  	export GIT_INDEX_VERSION
>>  fi
>
> Is this done a bit before ...
>
>> @@ -159,6 +159,7 @@ check_var_migration () {
>>  }
>>  
>>  check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
>
> ... this has a chance to kick in to say things like "Whoa you have
> TEST_GIT_INDEX_VERSION that is an old spelling of
> GIT_TEST_INDEX_VERSION", isn't it?

So, the obvious fix would look like the patch below.

One problem with warning is that

	$ TEST_GIT_INDEX_VERSION=4 sh ./t0000-basic.sh

(or any other depreated variable set without its modern counterpart
set) would fail due to extra output produced to the standard error
stream.

 t/test-lib.sh | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 17a56f44ad..8ef86e05a3 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 "${GIT_TEST_INDEX_VERSION:+isset}"
-then
-	GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
-	export GIT_INDEX_VERSION
-fi
-
 check_var_migration () {
 	old_name=$1 new_name=$2
 	eval "old_isset=\${${old_name}:+isset}"
@@ -162,6 +156,13 @@ 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}"
+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
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 0/5] Cleanup pass on special test setups
  2018-09-14 14:37 [PATCH v1 0/4] Cleanup pass on special test setups Ben Peart
                   ` (3 preceding siblings ...)
  2018-09-14 14:37 ` [PATCH v1 4/4] preload-index: update GIT_FORCE_PRELOAD_TEST support Ben Peart
@ 2018-09-18 23:29 ` Ben Peart
  2018-09-18 23:29   ` [PATCH v3 2/5] preload-index: use git_env_bool() not getenv() for customization Ben Peart
                     ` (5 more replies)
  4 siblings, 6 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-18 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, Ben Peart

This round has one code change based on feedback. Other changes are just
rewording commit messages.

Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/043246d936
Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v3 && git checkout 043246d936


### Interdiff (v2..v3):

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 17a56f44ad..8ef86e05a3 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 "${GIT_TEST_INDEX_VERSION:+isset}"
-then
-	GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
-	export GIT_INDEX_VERSION
-fi
-
 check_var_migration () {
 	old_name=$1 new_name=$2
 	eval "old_isset=\${${old_name}:+isset}"
@@ -162,6 +156,13 @@ 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}"
+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
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||


### Patches

Ben Peart (5):
  t/README: correct spelling of "uncommon"
  preload-index: use git_env_bool() not getenv() for customization
  fsmonitor: update GIT_TEST_FSMONITOR support
  read-cache: update TEST_GIT_INDEX_VERSION support
  preload-index: update GIT_FORCE_PRELOAD_TEST support

 Makefile                    |  6 +++---
 config.c                    |  2 +-
 preload-index.c             |  3 ++-
 t/README                    | 13 ++++++++++++-
 t/t1700-split-index.sh      |  2 +-
 t/t7519-status-fsmonitor.sh |  6 +++---
 t/test-lib.sh               | 27 +++++++++++++++++++++++++--
 7 files changed, 47 insertions(+), 12 deletions(-)


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
-- 
2.18.0.windows.1



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 1/5] t/README: correct spelling of "uncommon"
  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   ` Ben Peart
  2018-09-18 23:29   ` [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-18 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, Ben Peart, Ben Peart

Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 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
 <n> bytes.
 
-GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code
+GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code
 path where deltas larger than this limit require extra memory
 allocation for bookkeeping.
 
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 2/5] preload-index: use git_env_bool() not getenv() for customization
  2018-09-18 23:29 ` [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
@ 2018-09-18 23:29   ` Ben Peart
  2018-09-18 23:29   ` [PATCH v3 1/5] t/README: correct spelling of "uncommon" Ben Peart
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-18 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, Ben Peart, Ben Peart

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 <Ben.Peart@microsoft.com>
---
 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;
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
  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   ` Ben Peart
  2018-09-18 23:29   ` [PATCH v3 4/5] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-18 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, Ben Peart, Ben Peart

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.

Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---
 config.c                    |  2 +-
 t/README                    |  4 ++++
 t/t1700-split-index.sh      |  2 +-
 t/t7519-status-fsmonitor.sh |  2 +-
 t/test-lib.sh               | 20 ++++++++++++++++++++
 5 files changed, 27 insertions(+), 3 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=<n> 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..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
 # 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.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
 #
 
 # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
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 ||
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
  2018-09-18 23:29 ` [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
                     ` (2 preceding siblings ...)
  2018-09-18 23:29   ` [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support Ben Peart
@ 2018-09-18 23:29   ` 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   ` Re*: [PATCH v3 0/5] Cleanup pass on special test setups Junio C Hamano
  5 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-18 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, Ben Peart, Ben Peart

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 <Ben.Peart@microsoft.com>
---
 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=<n> 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
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support
  2018-09-18 23:29 ` [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
                     ` (3 preceding siblings ...)
  2018-09-18 23:29   ` [PATCH v3 4/5] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart
@ 2018-09-18 23:29   ` Ben Peart
  2018-09-20 18:43   ` Re*: [PATCH v3 0/5] Cleanup pass on special test setups Junio C Hamano
  5 siblings, 0 replies; 28+ messages in thread
From: Ben Peart @ 2018-09-18 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, Ben Peart, Ben Peart

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 <Ben.Peart@microsoft.com>
---
 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=<n> 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=<boolean> 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 d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,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}"
-- 
2.18.0.windows.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re*: [PATCH v3 0/5] Cleanup pass on special test setups
  2018-09-18 23:29 ` [PATCH v3 0/5] Cleanup pass on special test setups Ben Peart
                     ` (4 preceding siblings ...)
  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
  5 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2018-09-20 18:43 UTC (permalink / raw)
  To: Ben Peart; +Cc: git\, jrnieder\, Ben Peart

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"

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, back to index

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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   ` Re*: [PATCH v3 0/5] Cleanup pass on special test setups Junio C Hamano

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox