Re: [PATCH v2 01/14] t3705: test that 'sparse_entry' is unstaged
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Sep 15, 2021, 16:17 UTC
- Message-ID
- <ba372f28-3b90-db30-72fd-0242c1c1c3b3@gmail.com>
- In-Reply-To
- <CABPp-BF9rhAK47KQUJtZ9yDTkJ-qoeZ0Q7T4ycLcLSgLoxkzTw@mail.gmail.com>
On 9/15/2021 1:22 AM, Elijah Newren wrote:
Show 45 quoted lines
> On Sun, Sep 12, 2021 at 6:23 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> 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@microsoft.com>
>> ---
>> 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.
Show 11 quoted lines
> >> +} >> + >> 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.
Show 8 quoted lines
>> 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.
Show 7 quoted lines
>> 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.
Show 8 quoted lines
>> 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.
Show 8 quoted lines
>> 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