Rohit Ashiwal <[email protected]> writes:
> 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
Avoid making the log message into an enumerated list, when there
aren't that many things to enumerate to begin with (specifically,
the "test-lib-functions:" label is unsightly here). Finish the
sentence with a full stop.
Add a helper function to ensure that a given path is a
non-empty file, and give an error message when it is not.
Give separate messages for the case when the path is missing
or a non-file, and for the case when the path is a file but
is empty.
should be sufficient.
I still do not see why the posted code is better than this
if ! test -s "$1"
then
echo "'$1' is not a non-empty file.'
fi
which is more to the point. After all, if we are truly aiming for
finer-grained diagnosis, there is no good reason to accept a single
error message "does not exist or not a file" for these two cases,
but you'd be writing more like
if ! test -e "$1"
then
echo "'$1' does not exist"
elif ! test -f "$1"
then
echo "'$1' is not a file"
elif ! test -s "$1"
then
echo "'$1' is not empty"
else
: happy
return
fi
false
But I do not see much point in doing so, and I do not see much point
in the version that makes an extra check only for "test -f", either.
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 80402a428f..f9fcd2e013 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -593,6 +593,21 @@ test_dir_is_empty () {
> fi
> }
>
> +# Check if the file exists and has a size greater than zero
> +test_file_not_empty () {
> + if ! test -f "$1"
> + then
> + echo "'$1' does not exist or not a file."
> + false
> + else
> + if ! test -s "$1"
> + then
> + echo "'$1' is an empty file."
> + false
> + fi
> + fi
> +}
If I were writing this, I'd dedent it by turning this into
if ! test -f ...
then
echo ...
elif ! test -s ...
then
echo ...
else
: happy
return
fi
false
But as I said, I do not see much point in the extra "test -f", so
more likely this is what I would write, if I were doing this step
myself:
if test -s "$1"
then
: happy
else
echo "'$1' is not a non-empty file"
false
fi
> +
> test_path_is_missing () {
> if test -e "$1"
> then
