From: Christian Couder <christian.couder@gmail.com>
To: Ben Peart <peartben@gmail.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
Ben Peart <benpeart@microsoft.com>,
Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
David Turner <David.Turner@twosigma.com>,
Jeff King <peff@peff.net>
Subject: Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension
Date: Tue, 30 May 2017 15:18:49 +0200 [thread overview]
Message-ID: <CAP8UFD0OEUogQYL5Eo1kBy_1B=dBcdEJyc4B_qAUOU=2XnrxxA@mail.gmail.com> (raw)
In-Reply-To: <20170525183612.3128-5-benpeart@microsoft.com>
On Thu, May 25, 2017 at 8:36 PM, Ben Peart <peartben@gmail.com> wrote:
[...]
> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> new file mode 100755
> index 0000000000..395db46d55
> --- /dev/null
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -0,0 +1,158 @@
> +#!/bin/sh
> +
> +test_description='git status with file system watcher'
> +
> +. ./test-lib.sh
> +
> +clean_repo () {
> + git reset --hard HEAD
> + git clean -fd
> + rm -f marker
> +}
Maybe link all the commands in the function with "&&".
> +dirty_repo () {
> + : >untracked
> + : >dir1/untracked
> + : >dir2/untracked
> + echo 1 >modified
> + echo 2 >dir1/modified
> + echo 3 >dir2/modified
> + echo 4 >new
> + echo 5 >dir1/new
> + echo 6 >dir2/new
> + git add new
> + git add dir1/new
> + git add dir2/new
> +}
Idem.
> +# The test query-fsmonitor hook proc will output a marker file we can use to
> +# ensure the hook was actually used to generate the correct results.
> +
> +test_expect_success 'setup' '
> + mkdir -p .git/hooks &&
> + write_script .git/hooks/query-fsmonitor<<-\EOF &&
> + if [ $1 -ne 1 ]
> + then
> + echo -e "Unsupported query-fsmonitor hook version.\n" >&2
> + exit 1;
> + fi
> + : >marker
> + printf "untracked\0"
> + printf "dir1/untracked\0"
> + printf "dir2/untracked\0"
> + printf "modified\0"
> + printf "dir1/modified\0"
> + printf "dir2/modified\0"
> + printf "new\0""
> + printf "dir1/new\0"
> + printf "dir2/new\0"
Maybe something like the following to save a few lines and remove some
redundancies:
printf "%s\0" untracked dir1/untracked dir2/untracked \
modified dir1/modified dir2/modified \
new dir1/new dir2/new
or perhaps:
for f in untracked modified new
do
printf "%s\0" "$f" "dir1/$f" "dir2/$f"
done
> + EOF
> + : >tracked &&
> + : >modified &&
> + mkdir dir1 &&
> + : >dir1/tracked &&
> + : >dir1/modified &&
> + mkdir dir2 &&
> + : >dir2/tracked &&
> + : >dir2/modified &&
> + git add . &&
> + test_tick &&
> + git commit -m initial &&
> + dirty_repo
> +'
> +
> +cat >.gitignore <<\EOF
> +.gitignore
> +expect*
> +output*
> +marker*
> +EOF
This could be part of the previous setup test.
> +# Status is well tested elsewhere so we'll just ensure that the results are
> +# the same when using core.fsmonitor. First call after turning on the option
> +# does a complete scan so need to do two calls to ensure we test the new
> +# codepath.
> +
> +test_expect_success 'status with core.untrackedcache true' '
If this test is using untracked cache, it should perhaps first check
that untracked cache can be used on the current file system.
t7063-status-untracked-cache.sh does that with the following:
test_lazy_prereq UNTRACKED_CACHE '
{ git update-index --test-untracked-cache; ret=$?; } &&
test $ret -ne 1
'
if ! test_have_prereq UNTRACKED_CACHE; then
skip_all='This system does not support untracked cache'
test_done
fi
> + git config core.fsmonitor true &&
> + git config core.untrackedcache true &&
> + git -c core.fsmonitor=false -c core.untrackedcache=true status >expect &&
I don't understand why there is " -c core.untrackedcache=true" in the
above command as you already set core.untrackedcache to true on the
previous line.
> + clean_repo &&
> + git status &&
> + test_path_is_missing marker &&
> + dirty_repo &&
> + git status >output &&
> + test_path_is_file marker &&
> + test_i18ncmp expect output
> +'
> +
> +
Spurious new line.
> +test_expect_success 'status with core.untrackedcache false' '
> + git config core.fsmonitor true &&
> + git config core.untrackedcache false &&
> + git -c core.fsmonitor=false -c core.untrackedcache=false status >expect &&
Again core.untrackedcache is already set to false on the previous line.
> + clean_repo &&
> + git status &&
> + test_path_is_missing marker &&
> + dirty_repo &&
> + git status >output &&
> + test_path_is_file marker &&
> + test_i18ncmp expect output
> +'
> +
> +# Ensure commands that call refresh_index() to move the index back in time
> +# properly invalidate the fsmonitor cache
> +
> +test_expect_success 'refresh_index() invalidates fsmonitor cache' '
> + git config core.fsmonitor true &&
> + git config core.untrackedcache true &&
> + clean_repo &&
> + git status &&
> + test_path_is_missing marker &&
> + dirty_repo &&
> + write_script .git/hooks/query-fsmonitor<<-\EOF &&
> + :>marker
> + EOF
> + git add . &&
> + git commit -m "to reset" &&
> + git status &&
> + test_path_is_file marker &&
> + git reset HEAD~1 &&
> + git status >output &&
> + test_path_is_file marker &&
You already checked that "marker" exists 3 lines above, and as far as
I can see nothing could remove this file since the previous test, as
the hook can only create it.
So I wonder if something is missing or if this test is redundant.
> + git -c core.fsmonitor=false status >expect &&
> + test_i18ncmp expect output
> +'
> +
> +# Now make sure it's actually skipping the check for modified and untracked
> +# files unless it is told about them. Note, after "git reset --hard HEAD" no
> +# extensions exist other than 'TREE' so do a "git status" to get the extension
> +# written before testing the results.
> +
> +test_expect_success 'status doesnt detect unreported modifications' '
Maybe:
test_expect_success "status doesn't detect unreported modifications" '
> + git config core.fsmonitor true &&
> + git config core.untrackedcache true &&
> + write_script .git/hooks/query-fsmonitor<<-\EOF &&
> + :>marker
> + EOF
> + clean_repo &&
> + git status &&
> + test_path_is_missing marker &&
> + : >untracked &&
> + echo 2 >dir1/modified &&
> + git status >output &&
> + test_path_is_file marker &&
> + test_i18ngrep ! "Changes not staged for commit:" output &&
> + test_i18ngrep ! "Untracked files:" output &&
> + write_script .git/hooks/query-fsmonitor<<-\EOF &&
> + :>marker
> + printf "untracked%s\0"
Why is there a %s in the above?
> + printf "dir1/modified\0"
> + EOF
> + git status >output &&
> + test_path_is_file marker &&
> + test_i18ngrep "Changes not staged for commit:" output &&
> + test_i18ngrep "Untracked files:" output
> +'
> +
> +test_done
> --
> 2.13.0.windows.1.9.gc201c67b71
>
next prev parent reply other threads:[~2017-05-30 13:18 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-25 18:36 [PATCH v3 0/6] Fast git status via a file system watcher Ben Peart
2017-05-25 18:36 ` [PATCH v3 1/6] bswap: add 64 bit endianness helper get_be64 Ben Peart
2017-05-25 18:36 ` [PATCH v3 2/6] dir: make lookup_untracked() available outside of dir.c Ben Peart
2017-05-25 18:36 ` [PATCH v3 3/6] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files Ben Peart
2017-05-25 18:36 ` [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension Ben Peart
2017-05-30 13:18 ` Christian Couder [this message]
2017-05-30 21:21 ` Ben Peart
2017-05-30 22:37 ` Junio C Hamano
2017-05-31 0:10 ` Ben Peart
2017-05-31 4:33 ` Christian Couder
2017-05-31 14:57 ` Ben Peart
2017-05-25 18:36 ` [PATCH v3 5/6] fsmonitor: add documentation for the " Ben Peart
2017-05-25 18:36 ` [PATCH v3 6/6] fsmonitor: add a sample query-fsmonitor hook script for Watchman Ben Peart
2017-05-31 13:21 ` Christian Couder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAP8UFD0OEUogQYL5Eo1kBy_1B=dBcdEJyc4B_qAUOU=2XnrxxA@mail.gmail.com' \
--to=christian.couder@gmail.com \
--cc=David.Turner@twosigma.com \
--cc=benpeart@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=pclouds@gmail.com \
--cc=peartben@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).