| Message ID | 20230304025740.107830-1-cheskaqiqi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] t1092: add tests for `git diff-files` | expand |
On 3/3/2023 9:57 PM, Shuqi Liang wrote: > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -2054,5 +2054,37 @@ test_expect_success 'grep sparse directory within submodules' ' > git grep --cached --recurse-submodules a -- "*/folder1/*" >actual && > test_cmp actual expect > ' > +test_expect_success 'diff-files with pathspec inside sparse definition' ' nit: you need an empty line between the previous test's closing quote and the start of your new test. > + init_repos && > + > + write_script edit-contents <<-\EOF && > + echo text >>$1 > + EOF > + > + run_on_all ../edit-contents deep/a && > + > + test_all_match git diff-files && > + test_all_match git diff-files deep/a && > + test_all_match git diff-files --find-object=HEAD:a > +' > + > +test_expect_success 'diff-files with pathspec outside sparse definition' ' > + init_repos && > + > + write_script edit-contents <<-\EOF && > + echo text >>$1 > + EOF > + > + run_on_sparse mkdir newdirectory && > + run_on_sparse ../edit-contents newdirectory/testfile && > + test_sparse_match git sparse-checkout set newdirectory && > + test_sparse_match git add newdirectory/testfile && > + run_on_sparse ../edit-contents newdirectory/testfile && > + test_sparse_match git sparse-checkout set && These uses of 'git sparse-checkout set' are probably not necessary if you use "git add --sparse newdirectory/testfile". It does present an interesting modification of your test case: what if the file exists on-disk but outside of the sparse-checkout definition? What happens in each case? What if the file is different from the staged version? > + > + test_sparse_match git diff-files && > + test_sparse_match git diff-files newdirectory/testfile && > + test_sparse_match test_must_fail git diff-files --find-object=HEAD:testfile > +' These tests look like a good start here. I was first confused as to why you were doing such steps to modify the sparse-checkout definition, but I see it is critical that you have staged changes outside of the sparse-checkout cone. These kinds of details, the "why" you are doing subtle things, are great to add to the commit message. > To make sure git diff-files behaves as expected when > inside or outside of sparse-checkout definition. > > Add test for git diff-files: > Path is within sparse-checkout cone > Path is outside sparse-checkout cone With that in mind, here is a way you could edit your commit message to be more informative: Before integrating the 'git diff-files' builtin with the sparse index feature, add tests to t1092-sparse-checkout-compatibility.sh to ensure it currently works with sparse-checkout and will still work with sparse index after that integration. When adding tests against a sparse-checkout definition, we must test two modes: all changes are within the sparse-checkout cone and some changes are outside the sparse-checkout cone. In order to have staged changes outside of the sparse-checkout cone, create a 'newdirectory/testfile' and add it to the index, while leaving it outside of the sparse-checkout definition. (If you decide to add tests for the case of 'newdirectory/testfile' being present on-disk with or without modifications, then you can expand your commit message to include details about those tests, too.) Thanks, -Stolee
Turn on sparse-index feature within `git diff-files` command. Add necessary modifications and test them. Changes since v1: 1.Add an empty line between the previous test's closing quote and the start of new test. 2.Use "git add --sparse newdirectory/testfile" instead of 'git sparse-checkout set' to have staged changes outside of the sparse-checkout cone 3.Edit commit message to be more informative (sorry to send this patch twice ,I forgot to --inreply-to the origin one) Shuqi Liang (2): t1092: add tests for `git diff-files` diff-files: integrate with sparse index builtin/diff-files.c | 4 ++ t/perf/p2000-sparse-operations.sh | 2 + t/t1092-sparse-checkout-compatibility.sh | 52 ++++++++++++++++++++++++ 3 files changed, 58 insertions(+) base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 801919009e..f4815c619a 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -2054,5 +2054,37 @@ test_expect_success 'grep sparse directory within submodules' ' git grep --cached --recurse-submodules a -- "*/folder1/*" >actual && test_cmp actual expect ' +test_expect_success 'diff-files with pathspec inside sparse definition' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + run_on_all ../edit-contents deep/a && + + test_all_match git diff-files && + test_all_match git diff-files deep/a && + test_all_match git diff-files --find-object=HEAD:a +' + +test_expect_success 'diff-files with pathspec outside sparse definition' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + run_on_sparse mkdir newdirectory && + run_on_sparse ../edit-contents newdirectory/testfile && + test_sparse_match git sparse-checkout set newdirectory && + test_sparse_match git add newdirectory/testfile && + run_on_sparse ../edit-contents newdirectory/testfile && + test_sparse_match git sparse-checkout set && + + test_sparse_match git diff-files && + test_sparse_match git diff-files newdirectory/testfile && + test_sparse_match test_must_fail git diff-files --find-object=HEAD:testfile +' test_done
To make sure git diff-files behaves as expected when inside or outside of sparse-checkout definition. Add test for git diff-files: Path is within sparse-checkout cone Path is outside sparse-checkout cone Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com> --- t/t1092-sparse-checkout-compatibility.sh | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+)