t: test index is updated on checkout and pull

After our "git lfs checkout" and "git lfs pull" commands write the
contents of a Git LFS object into a file in the current working tree,
they pass the file's relative path to the "git update-index" command,
so the entries in Git's index for these files reflect their current
state.

If we did not run the "git update-index" command, Git commands like
"git diff-index" would report the files we updated as having been
modified in the working tree.

Notably, though, the "git status" command would normally not report the
files as modified, because it refreshes the Git index in the same way as
the "git update-index" command we run.  We invoke the "git update-index"
command with the --refresh option, so it runs the refresh_index()
function, which is also executed by the "git status" command.

The refresh_index() function checks the state of all the cached entries
in Git's index, and so it will detect that the files in the working
tree have been modified, read their content and pass it to the
"git lfs filter-process" command, which returns the same Git LFS
pointer data as should already be in the Git index.  The end result
is that the index entries remain unchanged, but the cached modification
timestamps are updated.

As a consequence, even if we never invoked the "git update-index"
command while running our "git lfs checkout" and "git lfs pull" commands,
a subsequent "git status" command would typically not show the working
tree files we update as modified.

However, in our tests of the "git lfs pull" command, we rely exclusively
on the "git status" command to try to verify that the "git update-index"
command has been executed by the "git lfs pull" command.  Moreover, the
assert_clean_status() test helper function we invoke to run the
"git status" command has a bug which means it always succeeds, even
if the "git status" command were to report that the current working
tree was not clean.

We added the assert_clean_status() assertion function in commit
7158e3b of PR git-lfs#2641, along with
calls to that function in the initial "pull" test of what is now
our t/t-pull.sh test script.  In the same commit we also revised the
"git lfs checkout" and "git lfs pull" commands so that they pass paths
relative to the current working directory to the "git update-index"
command, rather than paths relative to the root of the repository.

This latter change resolved the problem reported in git-lfs#2639, because
by passing file paths relative to the repository root instead of the
current working directory to the "git update-index" command, the
"git lfs checkout" and "git lfs pull" commands were leaving the Git
index and working tree in a confusing state.

At the time when git-lfs#2639 was reported, running "git lfs pull" in a
subdirectory might result in a subsequent "git status" command finding
modifications to both the Git index and the working tree.  To check
that the problem was resolved, in PR git-lfs#2641 we introduced the
assert_clean_status() test helper function, and expanded the "pull"
test to call this function after performing several "git lfs pull"
commands in subdirectories of the working tree.

Unfortunately, the assert_clean_status() function does not behave as
an assertion, because it always returns a successful (i.e., zero) exit
code.  The function tests whether the output from a "git status" command
includes the message "working tree clean", and if not, reports the
command's output along with that from a "git lfs status" command.
However, the function does not cause the current test's sub-shell to
stop execution and return a non-zero exit code to signal a test
failure condition.

We address this problem now by adding a call to the "exit" shell built-in
command with a non-zero exit code at the end of the brace-delimited
group command which executes if the assert_clean_status() function
does not find the "working tree clean" message in the output from the
"git status" command.  This change ensures that if Git determines that
the working tree is not clean, our assert_clean_status() function will
cause the test which called it to fail.

As well, we update the assert_clean_status() function so that when
checking for the "working tree clean" message in the output of the
"git status" command, it will also accept the older "working directory
clean" message output by the "git status" command prior to Git v2.10.0.
This allows our assertion function to succeed and the "pull" test to pass
in our GitHub Actions CI jobs that run our test suite against Git v2.0.0,
the oldest Git version we currently support.

As a further problem, though, checking that a "git status" command
finds a clean working tree is no longer sufficient to guarantee that
the "git lfs pull" command has actually called the "git update-index"
command with the appropriate file paths.  To be certain that the Git LFS
client has invoked the "git update-index" command and passed valid file
paths, we also need to check the output of the "git diff-index" command,
and do so before we run the "git status" command, which will cause Git
to reset the file modification timestamps it has cached.

We therefore define a new assert_clean_worktree() test helper function
which runs a "git diff-index HEAD" command and confirms no unexpected
changes are detected in the working tree, and we call this function at
the start of the assert_clean_status() function before the "git status"
command is executed.  This ensures that we check the state of the working
tree before the "git status" command refreshes the file modification
timestamps Git has previously cached.  As noted above, if we reversed
the order of these checks, the "git diff-index" command would always
return an empty list and so our check of the working tree's state would
be defeated.

We also define a new assert_clean_worktree_with_exceptions() test helper
function, which acts like the assert_clean_worktree() function but filters
the output of the "git diff-index HEAD" command with an extended regular
expression pattern provided to the function as its only argument.  This
allows us to use the assert_clean_worktree_with_exceptions() function in
instances where we expect certain files in the working tree to be absent
or modified and so we want to ignore them when they appear in the
"git diff-index" command's output.

We then update the initial "pull" test in the t/t-pull.sh test script
so that it calls either the assert_clean_status() function or the
assert_clean_worktree_with_exceptions() function after each invocation
of the "git lfs pull" command, thereby making the checks in this test
as thorough and consistent with each other as possible.

Finally, we update the initial "checkout" test in the t/t-checkout.sh
test script so that it makes use of the assert_clean_status() and
assert_clean_worktree_with_exceptions() functions in the same manner
as the "pull" test in the t/t-pull.sh script now does.  Both tests
perform similar operations, and like the "git lfs pull" command, the
"git lfs checkout" command also invokes the "git update-index" command.
However, the "checkout" test was not updated in PR git-lfs#2641 when the
assert_clean_status() was introduced and the "pull" test was updated
to call it.

Since we want our "checkout" test to verify that the "git lfs checkout"
command successfully runs the "git update-index" command, we update the
test so that it mirrors the "pull" test and consistently uses the
assert_clean_worktree() and assert_clean_worktree_with_exceptions()
functions to check the state of the working tree.

To ensure that the "checkout" test still passes, though, we also need to
adjust the test so that it removes any log files it creates because the
assert_clean_status() function now behaves as was originally intended and
will cause the calling test to fail if the working tree is not clean.

Author: chrisd8088

t/t-checkout.sh

@@ -10,9 +10,11 @@ begin_test "checkout"
   setup_remote_repo "$reponame"
 
   clone_repo "$reponame" repo
+  rm -f clone.log
 
   git lfs track "*.dat" 2>&1 | tee track.log
   grep "Tracking \"\*.dat\"" track.log
+  rm -f track.log
 
   contents="something something"
   contentsize=19
@@ -50,12 +52,15 @@ begin_test "checkout"
   grep "Checking out LFS objects: 100% (5/5), 95 B" checkout.log
   grep 'accepting "file1.dat"' checkout.log
   grep 'rejecting "file1.dat"' checkout.log && exit 1
+  rm -f checkout.log
+  assert_clean_status
 
   git rm file1.dat
 
   echo "checkout should skip replacing files deleted in index"
   git lfs checkout
   [ ! -f file1.dat ]
+  assert_clean_worktree_with_exceptions "file1\.dat"
 
   git reset --hard
 
@@ -69,13 +74,15 @@ begin_test "checkout"
   [ ! -f file3.dat ]
   [ ! -f folder1/nested.dat ]
   [ ! -f folder2/nested.dat ]
+  assert_clean_worktree_with_exceptions "(file[13]|nested)\.dat"
 
   echo "quotes to avoid shell globbing"
   git lfs checkout "file*.dat"
   [ "$contents" = "$(cat file1.dat)" ]
   [ "$contents" = "$(cat file3.dat)" ]
   [ ! -f folder1/nested.dat ]
   [ ! -f folder2/nested.dat ]
+  assert_clean_worktree_with_exceptions "nested\.dat"
 
   echo "test subdir context"
   rm file1.dat
@@ -84,40 +91,47 @@ begin_test "checkout"
     [ "$contents" = "$(cat nested.dat)" ]
     [ ! -f ../file1.dat ]
     [ ! -f ../folder2/nested.dat ]
+    assert_clean_worktree_with_exceptions "(file1|folder2/nested)\.dat"
 
     # test '.' in current dir
     rm nested.dat
-    git lfs checkout . 2>&1 | tee checkout.log
+    git lfs checkout .
     [ "$contents" = "$(cat nested.dat)" ]
     [ ! -f ../file1.dat ]
     [ ! -f ../folder2/nested.dat ]
+    assert_clean_worktree_with_exceptions "(file1|folder2/nested)\.dat"
 
     # test '..' in current dir
     git lfs checkout ..
     [ "$contents" = "$(cat ../file1.dat)" ]
     [ "$contents" = "$(cat ../folder2/nested.dat)" ]
+    assert_clean_status
 
     # test glob match with '..' in current dir
     rm -rf ../folder2
     git lfs checkout '../folder2/**'
     [ "$contents" = "$(cat ../folder2/nested.dat)" ]
+    assert_clean_status
   popd
 
   echo "test folder param"
   rm -rf folder2
   git lfs checkout folder2
   [ "$contents" = "$(cat folder2/nested.dat)" ]
+  assert_clean_status
 
   echo "test folder param with pre-existing directory"
   rm -rf folder2
   mkdir folder2
   git lfs checkout folder2
   [ "$contents" = "$(cat folder2/nested.dat)" ]
+  assert_clean_status
 
   echo "test folder param with glob match"
   rm -rf folder2
   git lfs checkout 'folder2/**'
   [ "$contents" = "$(cat folder2/nested.dat)" ]
+  assert_clean_status
 
   echo "test '.' in current dir"
   rm -rf file1.dat file2.dat file3.dat folder1/nested.dat folder2/nested.dat
@@ -127,6 +141,7 @@ begin_test "checkout"
   [ "$contents" = "$(cat file3.dat)" ]
   [ "$contents" = "$(cat folder1/nested.dat)" ]
   [ "$contents" = "$(cat folder2/nested.dat)" ]
+  assert_clean_status
 
   echo "test checkout with missing data doesn't fail"
   git push origin main
@@ -138,6 +153,7 @@ begin_test "checkout"
   [ "$(pointer $contents_oid $contentsize)" = "$(cat file3.dat)" ]
   [ "$contents" = "$(cat folder1/nested.dat)" ]
   [ "$contents" = "$(cat folder2/nested.dat)" ]
+  assert_clean_worktree_with_exceptions "file[123]\.dat"
 )
 end_test
 

t/t-pull.sh

@@ -82,6 +82,7 @@ begin_test "pull"
   assert_local_object "$contents_oid" 1
   assert_local_object "$contents2_oid" 1
   assert_local_object "$contents3_oid" 3
+  assert_clean_status
   git lfs fsck
 
   echo "lfs pull with remote"
@@ -154,6 +155,7 @@ begin_test "pull"
   assert_local_object "$contents_oid" 1
   refute_local_object "$contents2_oid"
   assert_local_object "$contents3_oid" 3
+  assert_clean_worktree_with_exceptions '\\303\\241\.dat'
 
   git lfs pull -I "*.dat"
   [ "A" = "$(cat "á.dat")" ]
@@ -198,6 +200,7 @@ begin_test "pull"
   refute_local_object "$contents_oid"
   assert_local_object "$contents2_oid" 1
   assert_local_object "$contents3_oid" 3
+  assert_clean_worktree_with_exceptions "a\.dat"
 
   pushd dir
     git lfs pull -I "*.dat"

t/testhelpers.sh

@@ -376,11 +376,24 @@ assert_hooks() {
   [ -x "$git_root/hooks/pre-push" ]
 }
 
+assert_clean_worktree() {
+  [ -z "$(git diff-index HEAD)" ]
+}
+
+assert_clean_worktree_with_exceptions() {
+  local exceptions="$1"
+
+  [ -z "$(git diff-index HEAD | grep -v -E "$exceptions")" ]
+}
+
 assert_clean_status() {
+  assert_clean_worktree
+
   status="$(git status)"
-  echo "$status" | grep "working tree clean" || {
+  echo "$status" | grep "working \(directory\|tree\) clean" || {
     echo $status
     git lfs status
+    exit 1
   }
 }