Re: [PATCH 1/3] test functions: Add new function `test_file_not_empty`

From: Junio C Hamano <gitster@pobox.com>
To: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Cc: git@vger.kernel.org, Johannes.Schindelin@gmx.de,
	t.gummerer@gmail.com, christian.couder@gmail.com
Subject: Re: [PATCH 1/3] test functions: Add new function `test_file_not_empty`
Date: Sun, 03 Mar 2019 22:20:13 +0900	[thread overview]
Message-ID: <xmqq5zt014du.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190303122842.30380-2-rohit.ashiwal265@gmail.com> (Rohit Ashiwal's message of "Sun, 3 Mar 2019 17:58:40 +0530")

Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:

> Subject: Re: [PATCH 1/3] test functions: Add new function `test_file_not_empty`

s/Add/add/.  Strictly speaking, you do not need to say "new", if you
are already saying "add", then that's redundant.

> test-lib-functions: add a helper function that checks for a file and that
> the file is not empty. The helper function will provide better error message
> in case of failure and improve readability
>
> Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
> ---
>  t/test-lib-functions.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 80402a428f..1302df63b6 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -593,6 +593,16 @@ test_dir_is_empty () {
>  	fi
>  }
>  
> +# Check if the file exists and has a size greater than zero
> +test_file_not_empty () {
> +	test_path_is_file "$1" &&
> +	if ! test -s "$1"

"test -s <path>" is true if <path> resolves to an existing directory
entry for a file that has a size greater than zero.  Isn't it
redundant and wasteful to have test_path_is_file before it, or is
there a situation where "test -s" alone won't give us what we want
to check?

> +	then
> +		echo "'$1' is an empty file."
> +		false
> +	fi
> +}
> +
>  test_path_is_missing () {
>  	if test -e "$1"
>  	then