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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2024, 22:11 UTC
Message-ID: <xmqqsf194n7a.fsf@gitster.g>
In-Reply-To: <20240301204922.40304-1-ericsunshine@charter.net>

Eric Sunshine <ericsunshine@charter.net> writes:

Show 9 quoted lines

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

Show 10 quoted lines

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