Subject: Re: [PATCH 2/4] show: integrate with the sparse index

From: Derrick Stolee <[email-masked]>
Date: Mon, 18 Apr 2022 08:28:41 -0400

On 4/14/2022 2:50 PM, Josh Steadmon wrote:
> On 2022.04.07 16:37, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <[email-masked]>

>>  	# 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