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

Eric Sunshine [off-list ref] writes:

quoted

+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.