Re: [PATCH v2 01/14] t3705: test that 'sparse_entry' is unstaged

On 9/15/2021 1:22 AM, Elijah Newren wrote:

On Sun, Sep 12, 2021 at 6:23 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

quoted

From: Derrick Stolee <redacted>

The tests in t3705-add-sparse-checkout.sh check to see how 'git add'
behaves with paths outside the sparse-checkout definition. These
currently check to see if a given warning is present but not that the
index is not updated with the sparse entries. Add a new
'test_sparse_entry_unstaged' helper to be sure 'git add' is behaving
correctly.

We need to modify setup_sparse_entry to actually commit the sparse_entry
file so it exists at HEAD but is not already staged in the index.

Signed-off-by: Derrick Stolee <redacted>
---
 t/t3705-add-sparse-checkout.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh
index 2b1fd0d0eef..af81b4b6846 100755
--- a/t/t3705-add-sparse-checkout.sh
+++ b/t/t3705-add-sparse-checkout.sh

@@ -19,6 +19,7 @@ setup_sparse_entry () {
        fi &&
        git add sparse_entry &&
        git update-index --skip-worktree sparse_entry &&
+       git commit --allow-empty -m "ensure sparse_entry exists at HEAD" &&
        SPARSE_ENTRY_BLOB=$(git rev-parse :sparse_entry)
 }

@@ -36,6 +37,11 @@ setup_gitignore () {
        EOF
 }

+test_sparse_entry_unstaged () {
+       git status --porcelain >actual &&
+       ! grep "^M  sparse_entry\$" actual

Is there a reason this is ^M rather than ^D?  Granted, both would be
bugs so I wouldn't expect either to appear, but the point of the check
is looking for likely errors.  Wouldn't the more likely error case for
a file not present in the working tree be that we stage the deletion
of the file?

You are right that we should be checking for deletions or adds
_as well_ as modifications. The test_sparse_entry_unstaged helper
is used in a variety of situations that typically would trigger
a modification, but at least one instance in this test is a
possible deletion.

quoted

+}
+
 test_expect_success 'setup' "
        cat >sparse_error_header <<-EOF &&
        The following pathspecs didn't match any eligible path, but they do match index

@@ -55,6 +61,7 @@ test_expect_success 'git add does not remove sparse entries' '
        setup_sparse_entry &&
        rm sparse_entry &&
        test_must_fail git add sparse_entry 2>stderr &&
+       test_sparse_entry_unstaged &&

Here, sparse_entry could be staged as a deletion.

quoted

        test_cmp error_and_hint stderr &&
        test_sparse_entry_unchanged
 '

@@ -73,6 +80,7 @@ test_expect_success 'git add . does not remove sparse entries' '
        rm sparse_entry &&
        setup_gitignore &&
        test_must_fail git add . 2>stderr &&
+       test_sparse_entry_unstaged &&

Deletion here.

quoted

        cat sparse_error_header >expect &&
        echo . >>expect &&

@@ -88,6 +96,7 @@ do
                setup_sparse_entry &&
                echo modified >sparse_entry &&
                test_must_fail git add $opt sparse_entry 2>stderr &&
+               test_sparse_entry_unstaged &&

But here would be modified.

quoted

                test_cmp error_and_hint stderr &&
                test_sparse_entry_unchanged
        '

@@ -98,6 +107,7 @@ test_expect_success 'git add --refresh does not update sparse entries' '
        git ls-files --debug sparse_entry | grep mtime >before &&
        test-tool chmtime -60 sparse_entry &&
        test_must_fail git add --refresh sparse_entry 2>stderr &&
+       test_sparse_entry_unstaged &&

Same here.

quoted

        test_cmp error_and_hint stderr &&
        git ls-files --debug sparse_entry | grep mtime >after &&
        test_cmp before after

@@ -106,6 +116,7 @@ test_expect_success 'git add --refresh does not update sparse entries' '
 test_expect_success 'git add --chmod does not update sparse entries' '
        setup_sparse_entry &&
        test_must_fail git add --chmod=+x sparse_entry 2>stderr &&
+       test_sparse_entry_unstaged &&

Here it would be modified with a mode change.

Using the pattern "^[MDARCU][M ] sparse_entry\$" seems to work while also
covering these other cases.

Thanks,
-Stolee