On Mon, Jul 12, 2021 at 6:56 AM Derrick Stolee <stolee@xxxxxxxxx> wrote: > > On 7/8/2021 9:03 PM, Elijah Newren wrote: > > On Mon, Jun 28, 2021 at 7:05 PM Derrick Stolee via GitGitGadget > > <gitgitgadget@xxxxxxxxx> wrote: > ... > >> +test_expect_success 'reset mixed and checkout orphan' ' > >> + init_repos && > >> + > >> + test_all_match git checkout rename-out-to-in && > >> + > >> + # Sparse checkouts do not agree with full checkouts about > >> + # how to report a directory/file conflict during a reset. > >> + # This command would fail with test_all_match because the > >> + # full checkout reports "T folder1/0/1" while a sparse > >> + # checkout reports "D folder1/0/1". This matches because > >> + # the sparse checkouts skip "adding" the other side of > >> + # the conflict. > > > > The same issue I highlighted last time is still present. If you > > insert an "exit 1" right here, then run > > ./t1092-sparse-checkout-compatibility.sh --ver --imm -x > > until it stops, then > > cd t/trash directory.t1092-sparse-checkout-compatibility/sparse-checkout > > git ls-files -t | grep folder # Note the files that are sparse > > git reset --mixed HEAD~1 > > git ls-files -t | grep folder # Note the files that are sparse -- > > there are some that aren't that should be > > git sparse-checkout reapply > > git ls-files -t | grep folder # Note the files that are sparse > > > > Granted, this is a bug with sparse-checkout without sparse-index, so > > not something new to your series. But since you are using comparisons > > between regular sparse-checkouts and sparse-index to verify > > correctness, this seems problematic to me. > > I'll add it to the pile, but I want to continue having this series > focus on making the sparse-index work quickly without a change in > behavior from a normal index. Changing the behavior of the sparse- > checkout feature should be a separate series. Hmm..perhaps there's some middle ground? I appreciate that you want to have this series focus on making the sparse-index work without worrying about behavioral changes to sparse-checkout. I'm concerned, though, that testcases tend to be treated as documentation of intended behavior, even when the tests are testing something else. These tests are clearly triggering buggy behavior, and I think your comments and subsequent command may be affected by it. I don't want to leave future folks (even ourselves) to have to try to explain away why the behavior expected in this test should not be expected. Perhaps we can just add a comment that this testcase is triggering a bug in both sparse-checkout and sparse-index but we're only checking that the two match, and that once the bug is fix, the testcase itself may need tweaking?
