| Message ID | 20230307065813.77059-2-cheskaqiqi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] t1092: add tests for `git diff-files` | expand |
Shuqi Liang <cheskaqiqi@gmail.com> writes: > diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh > index 801919009e..9382428352 100755 > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -2055,4 +2055,42 @@ test_expect_success 'grep sparse directory within submodules' ' > 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 (Documentation/CodingGuidelines) - Redirection operators should be written with space before, but no space after them. In other words, write 'echo test >"$file"' instead of 'echo test> $file' or 'echo test > $file'. Note that even though it is not required by POSIX to double-quote the redirection target in a variable (as shown above), our code does so because some versions of bash issue a warning without the quotes. > + #add file to the index but outside of cone Can you have a SP after "#" here to make it more readable? > + run_on_sparse mkdir newdirectory && > + run_on_sparse ../edit-contents newdirectory/testfile && > + test_sparse_match git add --sparse newdirectory/testfile && We create a new directory that is outside the cone, with or without using the sparse-index feature. We know we are violating the cone, and have to override the safety with the "--sparse" option. OK. What output do we expect out of "git add" to match in the two cases? > + #file present on-disk without modifications > + test_sparse_match git diff-files && > + test_sparse_match git diff-files newdirectory/testfile && As "diff-files" is about comparing between the index and the working tree, the new path should not appear in the output when the sparse checkout feature with or without the sparse-index feature is NOT in use. Does the picture get different when we are sparse? IOW, would we notice that we now have newdirectory/testfile that is supposed to be missing in the index and show that in the output? > + test_sparse_match test_must_fail git diff-files --find-object=HEAD:testfile && What does HEAD:testfile refer to in this test? This expects "diff-files" invocation to fail, and perhaps in your test it failed in both test repositories the same way, but are they failing for the right reason? In a non-sparse repository whose HEAD commit does not have 'testfile' (e.g. "git" source tree), I get $ git diff-files --find-object=HEAD:testfile error: unable to resolve 'HEAD:testfile' without sparse checkout or sparse index. It is unclear what value we get out of having this test here. > + #file present on-disk with modifications > + run_on_sparse ../edit-contents newdirectory/testfile && > + 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 Ditto. > +' > + > test_done Thanks.
Hi Junio On Tue, Mar 7, 2023 at 1:53 PM Junio C Hamano <gitster@pobox.com> wrote: > (Documentation/CodingGuidelines) > > - Redirection operators should be written with space before, but no > space after them. In other words, write 'echo test >"$file"' > instead of 'echo test> $file' or 'echo test > $file'. Note that > even though it is not required by POSIX to double-quote the > redirection target in a variable (as shown above), our code does so > because some versions of bash issue a warning without the quotes. Thanks for the styling reminders! I should go back and reread CodingGuidelines more often. > > + #add file to the index but outside of cone > > Can you have a SP after "#" here to make it more readable? Will do! > We create a new directory that is outside the cone, with or without > using the sparse-index feature. We know we are violating the cone, > and have to override the safety with the "--sparse" option. OK. > > What output do we expect out of "git add" to match in the two cases? > > > + #file present on-disk without modifications > > + test_sparse_match git diff-files && > > + test_sparse_match git diff-files newdirectory/testfile && > > As "diff-files" is about comparing between the index and the working > tree, the new path should not appear in the output when the sparse > checkout feature with or without the sparse-index feature is NOT in > use. Does the picture get different when we are sparse? IOW, would > we notice that we now have newdirectory/testfile that is supposed to > be missing in the index and show that in the output? I'm a bit caught up here. Do you mean I need to add a test for "git add" also? when we use "git add" instead of "git add --sparse ", we will get different. Cause newdirectory/testfile is missing in the index so diff-files will not work in these cases. > In a non-sparse repository whose HEAD commit does not have > 'testfile' (e.g. "git" source tree), I get > > $ git diff-files --find-object=HEAD:testfile > error: unable to resolve 'HEAD:testfile' > > without sparse checkout or sparse index. It is unclear what value > we get out of having this test here. Thanks for pointing out the error. HEAD:testfile is useless for the test here. ----------------------------------- Thanks Shuqi
Shuqi Liang <cheskaqiqi@gmail.com> writes: >> We create a new directory that is outside the cone, with or without >> using the sparse-index feature. We know we are violating the cone, >> and have to override the safety with the "--sparse" option. OK. >> >> What output do we expect out of "git add" to match in the two cases? >> >> > + #file present on-disk without modifications >> > + test_sparse_match git diff-files && >> > + test_sparse_match git diff-files newdirectory/testfile && >> >> As "diff-files" is about comparing between the index and the working >> tree, the new path should not appear in the output when the sparse >> checkout feature with or without the sparse-index feature is NOT in >> use. Does the picture get different when we are sparse? IOW, would >> we notice that we now have newdirectory/testfile that is supposed to >> be missing in the index and show that in the output? > > I'm a bit caught up here. > Do you mean I need to add a test for "git add" also? Not really. The above two tests are happy with _any_ output coming out of "git diff-files" (and "git diff-files nd/tf") as long as they match between sparse checkouts, one of which uses and the other does not use the sparse index feature. I was wondering if we want to be a bit stricter than that. Thinks like "not only the two output must match, they both must be empty".
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 801919009e..9382428352 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -2055,4 +2055,42 @@ test_expect_success 'grep sparse directory within submodules' ' 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 + + #add file to the index but outside of cone + run_on_sparse mkdir newdirectory && + run_on_sparse ../edit-contents newdirectory/testfile && + test_sparse_match git add --sparse newdirectory/testfile && + + #file present on-disk without modifications + 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 && + + #file present on-disk with modifications + run_on_sparse ../edit-contents newdirectory/testfile && + 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
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 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.Test 'newdirectory/testfile' being present on-disk without modifications, then change content inside 'newdirectory/testfile' in order to test 'newdirectory/testfile' being present on-disk with modifications. Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com> --- t/t1092-sparse-checkout-compatibility.sh | 38 ++++++++++++++++++++++++ 1 file changed, 38 insertions(+)