Re: [PATCH] test-lib-functions: simplify `test_file_not_empty` failure message
- From
Rubén Justo <rjusto@gmail.com>
- Date
- Mar 2, 2024, 16:38 UTC
- Message-ID
- <cf978790-4885-4103-946d-10f807048441@gmail.com>
- In-Reply-To
- <20240301204922.40304-1-ericsunshine@charter.net>
On Fri, Mar 01, 2024 at 03:49:22PM -0500, Eric Sunshine wrote:
Show 23 quoted lines
> From: Eric Sunshine <sunshine@sunshineco.com> > > The function `test_file_not_empty` asserts that a file exists and is not > empty. When the assertion fails, it complains: > > 'foo' is not a non-empty file. > > which is difficult to interpret due to the double-negative. To make it > easier to understand the problem, simplify the message by dropping the > double-negative and stating the problem more directly: > > 'foo' is empty but should not be > > (The full-stop is also dropped from the message to reflect the style of > messages issued by other `test_path_*` functions.) > > Note: Technically, the revised message is slightly less accurate since > the function asserts both that the file exists and that it is non-empty, > but the new message talks only about the emptiness of the file, not > whether it exists. A more accurate message might be "'foo' is empty but > should not be (or doesn't exist)", but that's unnecessarily long-winded > and adds little information that the test author couldn't discover by > noticing the file's absence.
To improve the accuracy of the message, I wonder if it is worth doing what we do in test_must_be_empty:
test_must_be_empty () {
test "$#" -ne 1 && BUG "1 param"
test_path_is_file "$1" &&
if test -s "$1"
then
echo "'$1' is not empty, it contains:"
cat "$1"
return 1
fi
}Perhaps:
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index b5eaf7fdc1..5b5ee0dc1d 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -989,9 +989,10 @@ test_dir_is_empty () { # Check if the file exists and has a size greater than zero test_file_not_empty () { test "$#" = 2 && BUG "2 param" + test_path_is_file "$1" && if ! test -s "$1" then - echo "'$1' is not a non-empty file." + echo "'$1' is empty but should not be" false fi } > > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> > --- > > This is a tangential follow-up to the discussion at [1]. > > [1]: https://lore.kernel.org/git/CAPig+cQ+JNBwydUq0CsTZGs8mHs3L3fJDuSosd+-WdKwWWw=gg@mail.gmail.com/ > > t/test-lib-functions.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > index b5eaf7fdc1..9e97b324c5 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -991,7 +991,7 @@ test_file_not_empty () { > test "$#" = 2 && BUG "2 param" > if ! test -s "$1" > then > - echo "'$1' is not a non-empty file." > + echo "'$1' is empty but should not be" > false > fi > } > -- > 2.44.0 >