I swear I'm not just on a mission to ruin everyone's GSOC projects. This patch definitely looks good, and given that we have this / document it makes sense.
However. I wonder in general if we've re-visited the utility of these wrappers and maybe other similar wrappers after -x was added.
Back when this was added in 2caf20c52b ("test-lib: user-friendly alternatives to test [-d|-f|-e]", 2010-08-10) we didn't have -x. So we'd at best fail like this:
$ ./t0001-init.sh -v -i
Initialized empty Git repository in /home/avar/g/git/t/trash directory.t0001-init/.git/
expecting success:
test -d .git &&
test -f doesnotexist &&
test -f .git/config not ok 1 - check files
#
# test -d .git &&
# test -f doesnotexist &&
# test -f .git/config
#At that point this was a definite improvement:
expecting success:
test_path_is_dir .git &&
test_path_is_file doesnotexist &&
test_path_is_file .git/config File doesnotexist doesn't exist.
not ok 1 - check filesBut 4 years after this was added in a136f6d8ff ("test-lib.sh: support -x option for shell-tracing", 2014-10-10) we got -x, and then with "-i -v -x":
expecting success:
test_path_is_dir .git &&
test_path_is_file doesnotexist &&
test_path_is_file .git/config + test_path_is_dir .git
+ test -d .git
+ test_path_is_file doesnotexist
+ test -f doesnotexist
+ echo File doesnotexist doesn't exist.
File doesnotexist doesn't exist.
+ false
error: last command exited with $?=1
not ok 1 - check filesBut by just using "test -d/-e": the much shorter:
+ test -d .git
+ test -f doesnotexist
error: last command exited with $?=1
not ok 1 - check filesSo I wonder if these days we shouldn't do this the other way around and get rid of these. Every test_* wrapper we add adds a bit of cognitive overload when you have to remember Git's specific shellscript dialect.
And at least to me whenever I have a test failure the first thing I do is try with -x (if I wasn't already using it). Under that the wrapper output is more verbose and no more helpful. It's immediately clear what's going on with:
+ test -f doesnotexist
error: last command exited with $?=1Whereas:
+ test -f doesnotexist
+ echo File doesnotexist doesn't exist.
File doesnotexist doesn't exist.
+ false
error: last command exited with $?=1Gives me the same thing, but I have to read 5 lines instead of 2 that ultimately don't tell me any more (and a bit of "huh, 'false' returned 1? Of course! Oh! It's faking things up and it's the 'echo' that matters...").
Looking over test-lib-functions.sh this patch would do it. I couldn't spot any other functions redundant to -x:
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 80402a428f..b3a95b4968 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -555,33 +555,6 @@ test_external_without_stderr () {
fi
} -# debugging-friendly alternatives to "test [-f|-d|-e]"
-# The commands test the existence or non-existence of $1. $2 can be
-# given to provide a more precise diagnosis.
-test_path_is_file () {
- if ! test -f "$1"
- then
- echo "File $1 doesn't exist. $2"
- false
- fi
-}
-
-test_path_is_dir () {
- if ! test -d "$1"
- then
- echo "Directory $1 doesn't exist. $2"
- false
- fi
-}
-
-test_path_exists () {
- if ! test -e "$1"
- then
- echo "Path $1 doesn't exist. $2"
- false
- fi
-}
-
# Check if the directory exists and is empty as expected, barf otherwise.
test_dir_is_empty () {
test_path_is_dir "$1" &&
@@ -593,19 +566,6 @@ test_dir_is_empty () {
fi
} -test_path_is_missing () {
- if test -e "$1"
- then
- echo "Path exists:"
- ls -ld "$1"
- if test $# -ge 1
- then
- echo "$*"
- fi
- false
- fi
-}
-
# test_line_count checks that a file has the number of lines it
# ought to. For example:
#
@@ -849,6 +809,9 @@ verbose () {
# otherwise. test_must_be_empty () {
+ # We don't want to remove this as noted in ec10b018e7 ("tests:
+ # use 'test_must_be_empty' instead of '! test -s'",
+ # 2018-08-19)
test_path_is_file "$1" &&
if test -s "$1"
then
Rohit Ashiwal via GitGitGadget
