On 9/15/2021 1:22 AM, Elijah Newren wrote:
> On Sun, Sep 12, 2021 at 6:23 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
>>
>> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>>
>> 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 <dstolee@xxxxxxxxxxxxx>
>> ---
>> 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.
>
>> +}
>> +
>> 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.
>> 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.
>> 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.
>> 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.
>> 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
