Re: [GSoC][PATCH v3 1/3] test functions: add function `test_file_not_empty`

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +test_file_not_empty () {
>> +       if ! test -s "$1"
>> +       then
>> +               echo "'$1' is not a non-empty file."
>
> Although not incorrect, the double-negative is hard to digest. I had
> to read it a few times to convince myself that it matched the intent
> of the new function. I wonder if a message such as
>
>     echo "'$1' is unexpectedly empty"
>
> would be better. (Subjective, and not at all worth a re-roll.)

Yeah, that is subjective.  The expectation of the test is "not-empty",
so I do not see this double-negation as being too bad, though.

> Much later in this same file, a function named test_must_be_empty() is
> defined, which is the complement of your new test_file_not_empty()
> function. The dissimilar names may cause confusion, so choosing a name
> more like the existing function might be warranted.
>
> Also, it might be a good idea to add this new function as a neighbor
> of test_must_be_empty() rather than defining it a couple hundred lines
> earlier in the file. Alternately, perhaps a preparatory patch could
> move test_must_be_empty() closer to the other similar functions
> (test_path_is_missing() and cousins).

Very good suggestions.  Looking at neighbouring helpers around
must-be-empty, it seems to me that the latter, i.e. moving it to sit
next to other "path" helpers, would make the most sense.

Thanks.