git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 4/6] fsmonitor: add test cases for fsmonitor extension

From
Christian Couder <christian.couder@gmail.com>
Date
May 30, 2017, 13:18 UTC
Message-ID
<CAP8UFD0OEUogQYL5Eo1kBy_1B=dBcdEJyc4B_qAUOU=2XnrxxA@mail.gmail.com>
In-Reply-To
<20170525183612.3128-5-benpeart@microsoft.com>
On Thu, May 25, 2017 at 8:36 PM, Ben Peart <peartben@gmail.com> wrote:
[...]
Show 17 quoted lines
> 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 "&&".
Show 14 quoted lines
> +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.
Show 21 quoted lines
> +# 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
Show 21 quoted lines
> +       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.
Show 6 quoted lines
> +# 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.

Show 10 quoted lines
> +       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.
Show 29 quoted lines
> +       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.

Show 10 quoted lines
> +       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" '
Show 17 quoted lines
> +       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?
Show 12 quoted lines
> +       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
>
Previous: Ben PeartNext: Ben Peart
Message 9 of 14 in “Fast git status via a file system watcher”
  1. 0/6 Fast git status via a file system watcherBen Peart, May 25, 2017
  2. 1/6 bswap: add 64 bit endianness helper get_be64Ben Peart, May 25, 2017
  3. 6/6 fsmonitor: add a sample query-fsmonitor hook script for WatchmanBen Peart, May 25, 2017
  4. Christian CouderMay 31, 2017
  5. 3/6 fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.Ben Peart, May 25, 2017
  6. 5/6 fsmonitor: add documentation for the fsmonitor extension.Ben Peart, May 25, 2017
  7. 2/6 dir: make lookup_untracked() available outside of dir.cBen Peart, May 25, 2017
  8. 4/6 fsmonitor: add test cases for fsmonitor extensionBen Peart, May 25, 2017
  9. Christian CouderMay 30, 2017
  10. Ben PeartMay 30, 2017
  11. Junio C HamanoMay 30, 2017
  12. Ben PeartMay 31, 2017
  13. Christian CouderMay 31, 2017
  14. Ben PeartMay 31, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.