| Message ID | 20180918232916.57736-4-benpeart@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] t/README: correct spelling of "uncommon" | expand |
On Tue, Sep 18, 2018 at 11:29:35PM +0000, Ben Peart wrote: > 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. Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we are good to go. > 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. But this old comment is different, suggesting copying that script to our $PATH. I prefer your instructions above, because it's only a single step, and, more importantly, it won't pollute my $PATH. I think this comment should be updated to make the advices in both places consistent. Or perhaps even removed, now that all GIT_TEST variables are documented in the same place?
On 9/28/2018 6:01 AM, SZEDER Gábor wrote: > On Tue, Sep 18, 2018 at 11:29:35PM +0000, Ben Peart wrote: >> 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. > > Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we > are good to go. > >> 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. > > But this old comment is different, suggesting copying that script to > our $PATH. > > I prefer your instructions above, because it's only a single step, > and, more importantly, it won't pollute my $PATH. I think this > comment should be updated to make the advices in both places > consistent. Or perhaps even removed, now that all GIT_TEST variables > are documented in the same place? > I prefer the suggestion to simply remove this text from the test script now that there is documentation for it in the t/README file.
On 9/28/2018 10:21 AM, Ben Peart wrote: > > > On 9/28/2018 6:01 AM, SZEDER Gábor wrote: >> On Tue, Sep 18, 2018 at 11:29:35PM +0000, Ben Peart wrote: >>> 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. >> >> Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we >> are good to go. >> >>> 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. >> >> But this old comment is different, suggesting copying that script to >> our $PATH. >> >> I prefer your instructions above, because it's only a single step, >> and, more importantly, it won't pollute my $PATH. I think this >> comment should be updated to make the advices in both places >> consistent. Or perhaps even removed, now that all GIT_TEST variables >> are documented in the same place? >> > > I prefer the suggestion to simply remove this text from the test script > now that there is documentation for it in the t/README file. Junio, can you squash in the following patch or would you prefer I reroll the entire series? Thanks, Ben From 393007340dc1baf3539ab727e0a8128e7c408a27 Mon Sep 17 00:00:00 2001 From: Ben Peart <peartben@gmail.com> Date: Fri, 28 Sep 2018 10:23:18 -0400 Subject: fixup! fsmonitor: remove outdated instructions from test 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 <peartben@gmail.com> --- Notes: Base Ref: git-test-cleanup-v3 Web-Diff: https://github.com/benpeart/git/commit/393007340d Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v1 && git checkout 393007340d t/t7519-status-fsmonitor.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 8308d6d5b1..3f0dd98010 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_TEST_FSMONITOR=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. base-commit: 043246d9369fb851c5c2b922466f77fc7ef0327b
Ben Peart <peartben@gmail.com> writes: > Junio, can you squash in the following patch or would you prefer I > reroll the entire series? Squash it to f8cd77d5 ("fsmonitor: update GIT_TEST_FSMONITOR support", 2018-09-18) and use the two new lines in the log message? I can do that. Thanks.
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 ||
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(-)