Re: [PATCH v3 1/5] t3701: redefine what is "bogus" output of a diff filter

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 31, 2022, 09:26 UTC
Message-ID: <742b42af-a298-219d-a35f-1fa8ba985aed@gmail.com>
In-Reply-To: <xmqq7d2pifby.fsf@gitster.g>

Hi Junio

On 30/08/2022 22:36, Junio C Hamano wrote:

Show 10 quoted lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> The existing test deletes the first line of the diff[1] - so it is
>> removing the "diff --git ..." header not the hunk header. This patch
>> changes the filter to delete everything except the diff header which
>> seems like a less realistic test.
> 
> Is it that all it cares about is that the output has the same number
> of lines as the input?  If so, I agree that it is much less realistic,
> but it may not matter in practice.  Even an "exit 0" might do ;-)

Yes I think it only cares that there are the same number of lines which begs the question "why are we changing this test in the first place?". The commit message talks about the being unable to parse the hunk header but that's only true because we would be looking in the wrong place for it - the output does in fact contain a valid hunk header. With this change there is no hunk header in the filtered output at all. In practice if the number of lines differs it is normally because the filter messed with the diff header and removed some lines from it which is what the existing test simulates.

I'm struggling to understand the need for this change from the explanation in the commit message as it seems to me to assume the line being deleted is the hunk header when in fact it is deleting the diff header.

Best Wishes

Phillip

> I may be quite off the mark on this one, though.
>