On 17/08/18 22:09, Junio C Hamano wrote:
> Andrei Rybak <rybak....@gmail.com> 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

Reply via email to