On 17/08/18 22:09, Junio C Hamano wrote:
> Andrei Rybak <rybak.a.v@xxxxxxxxx> writes:
>>
>> I'll try something like the following on the weekend:
>>
>> test_cmp () {
>> if test "$1" != - && ! test -s "$1"
>> then
>> echo >&4 "error: trying to compare empty file '$1'"
>> return 1
>> fi
>> if test "$2" != - && ! test -s "$2"
>> then
>> echo >&4 "error: trying to compare empty file '$2'"
>> return 1
>> fi
>> test_cmp_allow_empty "$@"
>> }
>
> I actually think the above gives way too confusing output, when the
> actual output is empty and we are expecting some output.
>
> The tester wants to hear from test_cmp "your 'git cmd' produced some
> output when we are expecting none" as the primary message. We are
> trying to find bugs in "git" under development, and diagnosing iffy
> tests is secondary. But with your change, the first thing that is
> checked is if 'expect' is an empty file and that is what we get
> complaints about, without even looking at what is in 'actual'.
I came up with two solutions for this issue:
1. Check both files at the same time (combination with Gábor's
function):
test_cmp () {
if test "$1" != - &&
test "$2" != - &&
! test -s "$1" &&
! test -s "$2"
then
error "bug in test script: using test_cmp to check empty file; use test_must_be_empty instead"
fi
test_cmp_allow_empty "$@"
}
This will still be reporting to the developer clearly, but
will only catch cases exactly like the bogus test in t5310.
2. Enable this check via variable, smth like EMPTY_CMP_LINT=1
