Andrei Rybak <rybak.a.v@xxxxxxxxx> writes:
> I think it would be a good trade-off to allow these helper functions to skip
> checking emptiness of arguments for test_cmp. Such patch will require only
> s/test_cmp/&_allow_empty/ for these helper functions and it will help catch
> cases as bogus test in t5310.
>
> I'll try something like the following on the weekend:
>
> test_cmp() {
Style: SP before and after ().
> 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. I.e.
: >expect &&
git cmd >actual &&
test_cmp expect actual
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'.
It's the same story, and it is even worse than the above, when we
are expecting some output but the command produced no output, i.e.
echo Everything up-to-date. >expect &&
git cmd >actual &&
test_cmp expect actual
We should get a complaint from test_cmp that 'actual' does not match
the string we were expecting (and even better, we show how they are
different by running them thru "diff -u"), not that 'actual' is an
empty file.
> test_cmp_allow_empty() {
> $GIT_TEST_CMP "$@"
> }
>
> (I'm not sure about redirections in test lib functions. The two if's would
> probably be in a separate function to be re-used by test_i18ncmp.)
