On 7/12/2021 3:32 PM, Elijah Newren wrote: > 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? I can get behind that approach: document the bug, but comment that it _is_ a bug and should be changed in the future. Thanks, -Stolee
