diff mbox series

t9801: replace test -f with test_path_is_file

Message ID 20210302185056.65929-1-shubhunic@gmail.com (mailing list archive)
State Accepted
Commit 12604a8d0c0a7d706c7be7be607a584260042ca9
Headers show
Series t9801: replace test -f with test_path_is_file | expand

Commit Message

Shubham Verma March 2, 2021, 6:50 p.m. UTC
Although `test -f` has the same functionality as test_path_is_file(), in
the case where test_path_is_file() fails, we get much better debugging
information.

Replace `test -f` with test_path_is_file so that future developers
will have a better experience debugging these test cases.

Signed-off-by: Shubham Verma <shubhunic@gmail.com>
---
 t/t9801-git-p4-branch.sh | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

Junio C Hamano March 4, 2021, 1:11 a.m. UTC | #1
Shubham Verma <shubhunic@gmail.com> writes:

> -		test -f file1 &&
> -		test -f file2 &&
> +		test_path_is_file file1 &&
> +		test_path_is_file file2 &&
>  		test ! -f file3 &&

You chose not to touch the last one, which is the right thing to do.
Replacing "test -f P" with "test_path_is_file P" can be done
mechanically, but it takes understanding of what the test is doing
to come up the correct replacement for "test ! -f P".  It may be
expecting P to be a directory, or it may be expecting P to be
missing, for example.

Will queue.  Thanks.
diff mbox series

Patch

diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 67ff2711f5..214c2a183d 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -200,19 +200,19 @@  test_expect_success 'git p4 clone simple branches' '
 		git p4 clone --dest=. --detect-branches //depot@all &&
 		git log --all --graph --decorate --stat &&
 		git reset --hard p4/depot/branch1 &&
-		test -f file1 &&
-		test -f file2 &&
-		test -f file3 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
 		grep update file2 &&
 		git reset --hard p4/depot/branch2 &&
-		test -f file1 &&
-		test -f file2 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
 		test ! -f file3 &&
 		! grep update file2 &&
 		git reset --hard p4/depot/branch3 &&
-		test -f file1 &&
-		test -f file2 &&
-		test -f file3 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
 		grep update file2 &&
 		cd "$cli" &&
 		cd branch1 &&
@@ -603,22 +603,22 @@  test_expect_success 'git p4 clone simple branches with base folder on server sid
 		git p4 clone --dest=. --use-client-spec  --detect-branches //depot@all &&
 		git log --all --graph --decorate --stat &&
 		git reset --hard p4/depot/branch1 &&
-		test -f file1 &&
-		test -f file2 &&
-		test -f file3 &&
-		test -f sub_file1 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
+		test_path_is_file sub_file1 &&
 		grep update file2 &&
 		git reset --hard p4/depot/branch2 &&
-		test -f file1 &&
-		test -f file2 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
 		test ! -f file3 &&
-		test -f sub_file1 &&
+		test_path_is_file sub_file1 &&
 		! grep update file2 &&
 		git reset --hard p4/depot/branch3 &&
-		test -f file1 &&
-		test -f file2 &&
-		test -f file3 &&
-		test -f sub_file1 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
+		test_path_is_file sub_file1 &&
 		grep update file2 &&
 		cd "$cli" &&
 		cd branch1 &&