Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> 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