Re: [PATCH 01/10] t1092: add tests for status/add and sparse files
From: Matheus Tavares Bernardino <hidden>
Date: 2021-04-21 15:15:14
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 [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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@mail.gmail.com/ (local)
+ 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