Re: [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash
From: Junio C Hamano <hidden>
Date: 2023-10-03 21:21:57
Štěpán Němec [off-list ref] writes:
quoted hunk
Turns out having `pwd` (which TEST_DIRECTORY defaults to) print $PWD with a trailing slash isn't very difficult, in my case it went something like ; tmux new-window -c ~/src/git/t/ [in the new window] ; sh ./t0000-basic.sh PANIC: Running in a /home/stepnem/src/git/t/ that doesn't end in '/t'? ; pwd /home/stepnem/src/git/t/ (tmux(1) apparently sets PWD in the environment in addition to calling chdir(2), which seems enough to make at least some shells preserve the trailing slash in `pwd` output.) Strip the trailing slash, if present, to prevent bailing out with the PANIC message in such cases. Signed-off-by: Štěpán Němec <redacted> --- t/test-lib.sh | 1 + 1 file changed, 1 insertion(+)diff --git a/t/test-lib.sh b/t/test-lib.sh index 1656c9eed006..3b6f1a17e349 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh@@ -35,6 +35,7 @@ else # needing to exist. TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1 fi +TEST_DIRECTORY="${TEST_DIRECTORY%/}" if test -z "$TEST_OUTPUT_DIRECTORY" then # Similarly, override this to store the test-results subdirbase-commit: d0e8084c65cbf949038ae4cc344ac2c2efd77415
While this would certainly squelch the particular test on TEST_DIRECTORY I am not sure if a safer fix would be to fix your PWD when it has a trailing slash. Your tests with this patch may be taking unintended branch when they do something like if test "$TRASH_DIRECTORY" = "$(pwd)" then ... fi as TRASH_DIRECTORY, whose default is derived from TEST_DIRECTORY and thanks to your fix it now does not have an extra slash in it, does not have a trailing slash but your $(pwd) or $PWD would have the trailing slash, simply because the patch only fixes TEST_DIRECTORY without fixing PWD. I wonder if this would be a safer alternative, or is it doing too much more than what is necessary? t/test-lib.sh | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
diff --git c/t/test-lib.sh w/t/test-lib.sh
index 1656c9eed0..6ec42eab0d 100644
--- c/t/test-lib.sh
+++ w/t/test-lib.sh@@ -22,19 +22,26 @@ then # ensure that TEST_DIRECTORY is an absolute path so that it # is valid even if the current working directory is changed TEST_DIRECTORY=$(pwd) -else - # The TEST_DIRECTORY will always be the path to the "t" - # directory in the git.git checkout. This is overridden by - # e.g. t/lib-subtest.sh, but only because its $(pwd) is - # different. Those tests still set "$TEST_DIRECTORY" to the - # same path. - # - # See use of "$GIT_BUILD_DIR" and "$TEST_DIRECTORY" below for - # hard assumptions about "$GIT_BUILD_DIR/t" existing and being - # the "$TEST_DIRECTORY", and e.g. "$TEST_DIRECTORY/helper" - # needing to exist. - TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1 fi + +# The TEST_DIRECTORY will always be the path to the "t" +# directory in the git.git checkout. This is overridden by +# e.g. t/lib-subtest.sh, but only because its $(pwd) is +# different. Those tests still set "$TEST_DIRECTORY" to the +# same path. +# +# See use of "$GIT_BUILD_DIR" and "$TEST_DIRECTORY" below for +# hard assumptions about "$GIT_BUILD_DIR/t" existing and being +# the "$TEST_DIRECTORY", and e.g. "$TEST_DIRECTORY/helper" +# needing to exist. +# +# Also, apparently a shell spawned in some tricky way can be +# talked into keeping a slash at the end of its $PWD. +# +# All of the above can be worked around by doing an extra chdir +# and asking where we ended up to be. +TEST_DIRECTORY=$(cd "$TEST_DIRECTORY/." && pwd) || exit 1 + if test -z "$TEST_OUTPUT_DIRECTORY" then # Similarly, override this to store the test-results subdir