Re: [PATCH v2] test-lib: user-friendly alternatives to test [!] [-d|-f]
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:15
Matthieu Moy wrote:
The helper functions are implemented, documented, and used in a few places to validate them
When I first read this, I thought you were saying these helpers already existed. This is where the rationale goes, anyway, so maybe: Add new test_file_must_not_exist et al helpers for use by tests to more loudly diagnose failures that manifest themselves by the existence or nonexistence of a file or directory. So now you can use test_file_must_exist foo "so there" from your test, and when it fails due to foo being absent or being a symlink instead, instead of silence you will get (if debugging with "-v") the helpful message file foo does not exist. so there.
quoted hunk ↗ jump to hunk
+++ b/t/README@@ -467,6 +467,14 @@ library for your script to use. <expected> file. This behaves like "cmp" but produces more helpful output when the test is run with "-v" option. + - test_file_must_exist <file> [<diagnosis>] + test_file_must_not_exist <file> [<diagnosis>] + test_dir_must_exist <dir> [<diagnosis>] + test_dir_must_not_exist <dir> [<diagnosis>] + + check whether a file/directory exists or doesn't. <diagnosis> will + be displayed if the test fails.
Maybe: - test_file_exists <name> [<diagnosis>] - test_dir_exists <name> [<diagnosis>] Check that <name> exists and is a file or directory, printing a diagnostic if it does not. The <diagnosis> if present will be used to give some added context to the diagnostic. - test_does_not_exist <name> [<diagnosis>] Check that <name> does not exist, printing a diagnostic if it does. The <diagnosis> will be printed on failure as added context if present. I think the ..._must_exist names put the emphasis in the wrong place, and they look funny in "if" statements.
quoted hunk ↗ jump to hunk
+++ b/t/t3404-rebase-interactive.sh +++ b/t/t3407-rebase-abort.sh
[examples] Makes sense.
quoted hunk ↗ jump to hunk
+++ b/t/test-lib.sh@@ -541,6 +541,38 @@ test_external_without_stderr () { fi } +# debugging-friendly alternatives to "test [!] [-f|-d]" +# The commands test the existence or non-existance of $1. $2 can be +# given to provide a more precise diagnosis. +test_file_must_exist () { + if ! [ -f "$1" ]; then + echo "file $1 doesn't exist. $*" + false + fi +}
Style nitpick: if statementss in the test-lib have tended to look like
if [ foo ]
then
bar
fi
so far. Here the whole function is a glorified "test -f", so I wonder
if
[ -f "$1" ] ||
{
echo >&2 "file $1 doesn't exist. $*"
false
}
would not be clearer. I dunno.
+test_file_must_not_exist () {
+ if [ -f "$1" ]; then
+ echo "file $1 exists. $*"
+ false
+ fi
+}What should happen if $1 exists and is not a file? I have often run into silent test failures of the sort your patch is designed to avoid. Thanks for tackling it.