On 4/14/2022 2:50 PM, Josh Steadmon wrote: > On 2022.04.07 16:37, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> # Asking "git show" for directories in the index >> - # does not work as implemented. The error message is >> - # different for a full checkout and a sparse checkout >> - # when the directory is outside of the cone. >> + # changes depending on the existence of a sparse index. > > The wording here seems awkward after these changes are applied. Without > other context, it makes it sound to me like the command(s) used to show > a directory change depending on the existence of a sparse index, rather > than the fact that the behavior of `git show` changes. I can see that. >> + # The output of "git show" includes the way we referenced the >> + # objects, so strip that out. >> + test_line_count = 4 actual && >> + tail -n 2 actual >actual-trunc && >> + tail -n 2 expect >expect-trunc && >> + test_cmp expect-trunc actual-trunc >> ' > > It's not specific to this commit, but in general I think the series of > changes to this test would be easier to follow if we used hard-coded > strings to compare against, rather than matching parts of files against > each other. It makes it more clear to the reader exactly which behavior > is changing, and can make it more obvious why certain output is > undesirable. However, it would make the test more brittle to future > changes. The tests here are designed with this approach in mind: demonstrate success by comparing to existing behavior. We don't want to be coupled to the exact behavior of these commands, but we _do_ want to demonstrate that the sparse-checkout or sparse-index features do not change from the full-checkout behavior (unless we are demonstrating an expected difference). In particular, using comparisons like this are also robust against changes in the test repository data shape, which has been necessary to update as bugs are found. Thanks, -Stolee