Re: [PATCH 1/1] quote: handle null and empty strings in sq_quote_buf_pretty()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 21, 2019, 15:22 UTC
Message-ID: <xmqq7e76tufs.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqqtvabtwai.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 7 quoted lines

> "Garima Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>>  create mode 100644 t/helper/test-quote.c
>>  create mode 100755 t/t0091-quote.sh
>
> I do not appreciate these two new files only to test this corner
> case.  ...

To avoid misunderstanding, I am not against having unit tests when they are appropriate. What I am against is to have only unit tests, especially when the effect of a bug (and its fix) can be tested with externally observable behaviour. The latter gives us a better sense of the real-world impact (e.g. if run_command would spawn the given command via shell using the 'sh -c "... stringified command and its arguments ..."' idiom, it may be done with the function we fixed here, which would mean that the user cannot pass '' as an argument to that codepath), while a unit test gives readers "ok, the function behaves that way now" alone, without answering "then what? What difference does this fix make to my use of Git as a whole?".

In any case, thanks for an attempt to fix.