git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Rubén Justo <rjusto@gmail.com>
Date
Mar 2, 2024, 16:38 UTC
Message-ID
<cf978790-4885-4103-946d-10f807048441@gmail.com>
In-Reply-To
<20240301204922.40304-1-ericsunshine@charter.net>
On Fri, Mar 01, 2024 at 03:49:22PM -0500, Eric Sunshine wrote:
Show 23 quoted lines
> 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
> 
Previous: Dirk GoudersNext: Junio C Hamano
Message 7 of 8 in “test-lib-functions: simplify `test_file_not_empty` failure message”
  1. test-lib-functions: simplify `test_file_not_empty` failure messageEric Sunshine, Mar 1, 2024
  2. Junio C HamanoMar 1, 2024
  3. Eric SunshineMar 1, 2024
  4. Dirk GoudersMar 2, 2024
  5. Junio C HamanoMar 2, 2024
  6. Dirk GoudersMar 3, 2024
  7. Rubén JustoMar 2, 2024
  8. Junio C HamanoMar 2, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.