Re: [PATCH v4] add-patch: response to unknown command
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Apr 29, 2024, 09:48 UTC
- Message-ID
- <33bb69ad-ec79-4863-97b7-51605b94c9c5@gmail.com>
- In-Reply-To
- <xmqqfrva22py.fsf@gitster.g>
Hi Junio
On 24/04/2024 16:35, Junio C Hamano wrote:
Show 38 quoted lines
> phillip.wood123@gmail.com writes:
>
>> On 22/04/2024 16:50, Junio C Hamano wrote:
>>> Rubén Justo <rjusto@gmail.com> writes:
>>>
>>>> 1: 0317594bce ! 1: b418b03f15 add-patch: response to unknown command
>>>> @@ t/t3701-add-interactive.sh: test_expect_success 'warn about add.interactive.useB
>>>> + test_when_finished "git reset --hard; rm -f command" &&
>>>> + echo W >command &&
>>>> + git add -N command &&
>>>> -+ cat >expect <<-EOF &&
>>>> -+ diff --git a/command b/command
>>>> -+ new file mode 100644
>>>> -+ index 0000000..a42d8ff
>>>> -+ --- /dev/null
>>>> -+ +++ b/command
>>>> -+ @@ -0,0 +1 @@
>>>> -+ +W
>>>> ++ git diff command >expect &&
>>>> ++ cat >>expect <<-EOF &&
>>>> + (1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
>>>> + (1/1) Stage addition [y,n,q,a,d,e,p,?]?$SP
>>>> + EOF
>>> Interesting.
>>> My first reaction was "how is this different from checking just the
>>> last line of the actual output? The early part of expect and actual
>>> both come from an internal invocation of 'git diff', and they must
>>> match by definition".
>>> But that may really be the point of this test.
>>
>> Yes - we want to make sure that we are not printing the help and the
>> only way to do that is to check the whole output
>
> I was not questioning that part of the patch. "My first reaction"
> was solely about use of "git diff" to replace the golden copy of
> expected result in the test itself, only to allow for use of
> different hash functions. As you (or somebody else?) mentioned in
> an earlier review, diff_cmp is there for exactly that purpose.Oh sorry I'd misunderstood
Show 12 quoted lines
>>> That is, we may later decide to tweak the way "git diff" hunks are >>> presented, and we expect that the way "git add -p" presents the >>> hunks would change together with it automatically. > > This argument cuts both ways, by the way. > > Insisting that the output match the explicit expectation (not what > "git diff" of the day produces) has a few advantages. It makes the > test more explicit and easy to see what output we are expecting, and > more importantly, it forces us to update the test when we decide to > tweak the output from the command being tested (i.e. hunk selection > UI) and/or the output from "git diff" command.
There is also a practical argument against using "git diff" to generate the expected output as it only works if the diff contains a single hunk. If the diff contains more than one hunk "add -p" displays them separately.
Best Wishes
Phillip