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.
> +++ 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.
> +++ b/t/t3404-rebase-interactive.sh
> +++ b/t/t3407-rebase-abort.sh
[examples]
Makes sense.
> +++ 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html