Thread (1 message) 1 message, 1 author, 2024-02-12

Re: [PATCH v2] t9146: replace test -d/-e/-f with appropriate test_path_is_* function

From: Junio C Hamano <hidden>
Date: 2024-02-12 20:31:05

"Chandra Pratap via GitGitGadget" [off-list ref] writes:
From: Chandra Pratap <redacted>

The helper functions test_path_is_* provide better debugging
information than test -d/-e/-f.
Correct.
Replace "if ! test -d then <error message>" with "test_path_exists"
and "test -d" with "test_path_is_dir" at places where we check for
existent directories.
The former could result in misconversion, if the intention of the
test was "we cannot have directory here; a regular file is OK", so
we have to be a bit more careful than mechanical conversion.
Replace "test -f" with "test_path_is_file" at places where we check
for existent files.
OK.
Replace "test ! -e" with "test_path_is_missing" where we check for
non-existent directories.
OK.
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_exists "$i" || exit 1
We were saying that we are OK if "$i" existed as a file (not a
directory), but now we complain regardless of what "$i" is.  Is that
closer to what the test originally wanted to do?  Just checking.
quoted hunk
 		done
 	)
 '
@@ -37,11 +33,7 @@ test_expect_success 'option automkdirs set to false' '
 		git svn fetch &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if test -d "$i"
-			then
-				echo >&2 "$i exists" &&
-				exit 1
-			fi
+			test_path_is_missing "$i" || exit 1
Ditto; are we sure the intention of the original is that nothing
should be at "$i" (instead of "as long as it is not a directory,
we are OK")?  Just checking.

The same comment applies to all conversions to test_path_exists and
test_path_is_missing where the original was not "test -e" or "! test -e".
The other ones, like the change from "test -f" to "test_path_is_file",
looked all correct.

Thanks.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help