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

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

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.
 

quoted

+       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.

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/ (local)

Can do!

quoted

+       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