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. On Tue, Apr 13, 2021 at 11:02 AM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh > index 12e6c453024f..6598c12a2069 100755 > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -232,6 +232,42 @@ test_expect_success 'add, commit, checkout' ' > test_all_match git checkout - > ' > > +test_expect_success 'status/add: outside sparse cone' ' > + init_repos && > + > + # folder1 is at HEAD, but outside the sparse cone > + run_on_sparse mkdir folder1 && > + cp initial-repo/folder1/a sparse-checkout/folder1/a && > + cp initial-repo/folder1/a sparse-index/folder1/a && > + > + test_sparse_match git status && > + > + write_script edit-contents <<-\EOF && > + echo text >>$1 > + EOF > + run_on_all ../edit-contents folder1/a && > + run_on_all ../edit-contents folder1/new && > + > + test_sparse_match git status --porcelain=v2 && > + > + # This "git add folder1/a" is completely ignored > + # by the sparse-checkout repos. It causes the > + # full repo to have a different staged environment. > + > + 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 ? 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@xxxxxxxxxxxxxx/ > + 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? > + test_all_match git add . && > + test_all_match git status --porcelain=v2 && > + test_all_match git commit -m folder1/new && > + > + run_on_all ../edit-contents folder1/newer && > + test_all_match git add folder1/ && > + test_all_match git status --porcelain=v2 && > + test_all_match git commit -m folder1/newer > +' > + > test_expect_success 'checkout and reset --hard' ' > init_repos && > > -- > gitgitgadget >
