shejialuo <shejialuo@xxxxxxxxx> writes:
> test_expect_success 'basic clone' '
> - test ! -d trunk &&
> + ! test_path_is_dir trunk &&
This is not quite right. Step back and think why we are trying to
use the test_path_* helpers instead of "test [!] -d". What are the
differences between them?
The answer is that, unlike "test [!] -d dir" that is silent whether
"dir" exists or missing, "test_path_is_dir dir" is *not* always
silent. It gives useful messages as necessary. When does it do so?
Here is the definition, from t/test-lib-functions.sh around line
930:
test_path_is_dir () {
test "$#" -ne 1 && BUG "1 param"
if ! test -d "$1"
then
echo "Directory $1 doesn't exist"
false
fi
}
It succeeds silently when "test -d dir" is true, but it complains
loudly when "test -d dir" does not hold. You will be told that the
test is unhappy because "dir" does not exist. That would be easier
to debug than one step among many in &&-chain silently fails.
Now, let's look at the original you rewrote again:
> - test ! -d trunk &&
It says "it is a failure if 'trunk' exists as a directory". If
'trunk' does not exist, it is a very happy state for us. So instead
of silently failing when 'trunk' exists as a directory, you would
want to improve it so that you will get a complaint in such a case,
saying "trunk should *not* exist but it does".
Did you succeed to do so with this rewrite?
> + ! test_path_is_dir trunk &&
The helper "test_path_is_dir" is called with "trunk". As we saw, we
will see complaint when "trunk" does *NOT* exist. When "trunk" does
exist, it will be silent and "test_path_is_dir" will return a success,
which will be inverted with "!" to make it a failure, causing &&-chain
to fail.
So the exit status is not wrong, but it issues a complaint under the
wrong condition. That is not an improvement.
Let's step back one more time. Is the original test happy when
"trunk" existed as a regular file? "test ! -d trunk" says so, but
should it really be? Think.
I suspect that the test is not happy as long as 'trunk' exists,
whether it is a directory or a regular file or a symbolic link.
IOW, it says "I am unhappy if 'trunk' is a directory", but what it
really meant to say was "I am unhappy if there is anything at the
path 'trunk'". IOW, "test ! -e trunk" would be what it really
meant, no?
So the correct rewrite for it would rather be something like
test_path_is_missing trunk &&
instead. This will fail if anything is at path 'trunk', with an
error message saying there shouldn't be anything but there is.
In a peculiar case, which I do not think this one is, a test may
legitimately accept "path" to either (1) exist as long as it is not
a directory, or (2) be missing, as success. In such a case, the
original construct '! test -d path" (or "test ! -d path") would be
appropriate.
But I do not think we have a suitable wrapper to express such a
case, i.e. we do not have a helper like this.
test_path_is_not_dir () {
if test -d "$1"
then
echo "$1 is a directory but it should not be"
false
fi
}
If such a use case were common, we might even do this:
# "test_path_is_dir <dir>" expects <dir> to be a directory.
# "test_path_is_dir ! <dir>" expects <dir> not to be a
# directory.
# In either case, complain only when the expectation is not met.
test_path_is_dir () {
if test "$1" = "!"
then
shift
if test -d "$1"
then
echo "$1 is a directory but it should not be"
return 1
fi
else
if test ! -d "$1"
then
echo "$1 is not a directory"
return 1
fi
fi
true
}
but "we are happy even if path exists as long as it is not a
directory" is a very uncommon thing we want to say in our tests, so
that is why we do not have such a helper function.
HTH.
