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

On Thu, May 25, 2017 at 8:36 PM, Ben Peart [off-list ref] wrote:

[...]

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