| Message ID | 20190303233750.6500-2-rohit.ashiwal265@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use helper functions in test script | expand |
Rohit Ashiwal <rohit.ashiwal265@gmail.com> 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
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 +} + test_path_is_missing () { if test -e "$1" then
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 The function `test_file_not_empty`, first checks if a file is provided, if it is not then an error message is printed, skipping the remaining code, if <path> is indeed a file then check `test -s` is applied to check if size of file is greater than zero, failing which another error message is printed. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> --- t/test-lib-functions.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+)