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

["Rubén Justo" <rjusto@gmail.com>]

On Fri, Mar 01, 2024 at 03:49:22PM -0500, Eric Sunshine wrote:
> From: Eric Sunshine <sunshine@sunshineco.com>
> 
> The function `test_file_not_empty` asserts that a file exists and is not
> empty. When the assertion fails, it complains:
> 
>     'foo' is not a non-empty file.
> 
> which is difficult to interpret due to the double-negative. To make it
> easier to understand the problem, simplify the message by dropping the
> double-negative and stating the problem more directly:
> 
>     'foo' is empty but should not be
> 
> (The full-stop is also dropped from the message to reflect the style of
> messages issued by other `test_path_*` functions.)
> 
> Note: Technically, the revised message is slightly less accurate since
> the function asserts both that the file exists and that it is non-empty,
> but the new message talks only about the emptiness of the file, not
> whether it exists. 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.

To improve the accuracy of the message, I wonder if it is worth doing
what we do in test_must_be_empty:

	test_must_be_empty () {
		test "$#" -ne 1 && BUG "1 param"
		test_path_is_file "$1" &&
		if test -s "$1"
		then
			echo "'$1' is not empty, it contains:"
			cat "$1"
			return 1
		fi
	}

Perhaps:

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b5eaf7fdc1..5b5ee0dc1d 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -989,9 +989,10 @@ test_dir_is_empty () {
 # Check if the file exists and has a size greater than zero
 test_file_not_empty () {
        test "$#" = 2 && BUG "2 param"
+       test_path_is_file "$1" &&
        if ! test -s "$1"
        then
-               echo "'$1' is not a non-empty file."
+		echo "'$1' is empty but should not be"
                false
        fi
 }

> 
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
> 
> This is a tangential follow-up to the discussion at [1].
> 
> [1]: https://lore.kernel.org/git/CAPig+cQ+JNBwydUq0CsTZGs8mHs3L3fJDuSosd+-WdKwWWw=gg@mail.gmail.com/
> 
>  t/test-lib-functions.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index b5eaf7fdc1..9e97b324c5 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -991,7 +991,7 @@ test_file_not_empty () {
>  	test "$#" = 2 && BUG "2 param"
>  	if ! test -s "$1"
>  	then
> -		echo "'$1' is not a non-empty file."
> +		echo "'$1' is empty but should not be"
>  		false
>  	fi
>  }
> -- 
> 2.44.0
>