Re: [PATCH 01/10] t1092: add tests for status/add and sparse files

From: Derrick Stolee <stolee@gmail.com>
Date: Apr 23, 2021, 20:12 UTC
Message-ID: <94a175eb-8b1a-638e-df85-2d33713ab4fb@gmail.com>
In-Reply-To: <CAHd-oW4wq6T1kmBGXUfEO4R8h0dfUAGudKvO0xLgCgm2bQt=qA@mail.gmail.com>

On 4/21/2021 11:14 AM, Matheus Tavares Bernardino wrote:

Show 6 quoted lines

> Hi, Stolee
> 
> You already said you will make changes in this test to make sure
> git-add's sparse warning is kept on a sparse index (BTW thanks for
> that :), but I just wanted to give a couple suggestions that came to
> my mind while reading the patch.

I appreciate the suggestions! More tests always help me from
making mistakes, and you are definitely more of a 'git add'
expert than me.

Show 6 quoted lines

>> +       test_must_fail git -C sparse-checkout add folder1/a &&
>> +       test_must_fail git -C sparse-index add folder1/a &&
> 
> To make sure the output is the same, could we collapse these two lines into:
> 
> test_sparse_match test_must_fail git add folder1/a ?

This is elegant. I'm sad I didn't think of it earlier.

Show 8 quoted lines

> And additionally, I think we could repeat this check with `add
> --refresh` and also after removing `folder1/a`. The reason I'm saying
> this is because the check currently succeeds when `folder1/a` is in
> the working tree (maybe because `fill_directory()` ends up expanding
> the sparse index in this case?), but not under the two other
> circumstances I mentioned (as we've discussed in [1]).
> 
> [1]: https://lore.kernel.org/git/CAHd-oW7vCKC-XRM=rX37+jQn_XDzjtar9nNHKQ-4OHSZ=2=KFA@mail.gmail.com/

Can do!

Show 6 quoted lines

>> +       git -C full-checkout checkout HEAD -- folder1/a &&
>> +       test_sparse_match git status --porcelain=v2 &&
> 
> Hmm, shouldn't this be `test_all_match`? IIUC, we've resetted
> `folder1/a` on the full repo to make sure the status report is the
> same across all repos, right?

Yes!

Thanks, -Stolee