On Mon, Mar 4, 2019 at 7:08 AM Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> wrote: > Add a helper function to ensure that a given path is a non-empty file, > and give an error message when it is not. > > Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> > --- > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > @@ -593,6 +593,15 @@ test_dir_is_empty () { > +# Check if the file exists and has a size greater than zero > +test_file_not_empty () { > + if ! test -s "$1" > + then > + echo "'$1' is not a non-empty file." Although not incorrect, the double-negative is hard to digest. I had to read it a few times to convince myself that it matched the intent of the new function. I wonder if a message such as echo "'$1' is unexpectedly empty" would be better. (Subjective, and not at all worth a re-roll.) > + false > + fi > +} > test_path_is_missing () { Much later in this same file, a function named test_must_be_empty() is defined, which is the complement of your new test_file_not_empty() function. The dissimilar names may cause confusion, so choosing a name more like the existing function might be warranted. Also, it might be a good idea to add this new function as a neighbor of test_must_be_empty() rather than defining it a couple hundred lines earlier in the file. Alternately, perhaps a preparatory patch could move test_must_be_empty() closer to the other similar functions (test_path_is_missing() and cousins).