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

From: Junio C Hamano <[email-masked]>
Date: Fri, 01 Mar 2024 14:11:21 -0800

Eric Sunshine <[email-masked]> writes:

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

Besides, that is way too confusing.  "<foo> is empty or it does not
exist" I may understand, but with your construct, I wouldn't be able
to tell how I am supposed to interpret the "(or doesn't exist)"
part.

> 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"

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.