Re: [PATCH] test-lib-functions: simplify `test_file_not_empty` failure message

Eric Sunshine [off-list ref] writes:

On Fri, Mar 1, 2024 at 5:11 PM Junio C Hamano [off-list ref] wrote:

quoted

Eric Sunshine [off-list ref] writes:

quoted

A more accurate message might be "'foo' is empty but
should not be (or doesn't exist)", but that's unnecessarily long-winded
and adds little information that the test author couldn't discover by
noticing the file's absence.

The "adds little information" version may be

                echo "'$1' is either missing or empty, but should not be"

And avoiding "X is Y, but should  be ~Y" construct, perhaps

                echo "'$1' should be a file with non-empty contents"

would work better?  I dunno.

I find "'$1' is either missing or empty, but should not be" suggestion
clear and easily understood. I'll reroll with that.

This is a view from a position with more distance:

I find that not so easily understood -- the "but should not
be" part is rather unexpected and I feel, it doesn't provide necessary
information, e.g.:

test_path_is_executable () {
...
		echo "$1 is not executable"
...

also doesn't state what is wanted and I doubt that message doesn't
clearly describe the problem.

While I looked at it: there is another `test -s` in test_grep () that
perhaps could be fixed the same way:

	if test -s "$last_arg"
	then
		cat >&4 "$last_arg"
	else
		echo >&4 "<File '$last_arg' is empty>"
	fi

Dirk