[PATCH] t9801: replace test -f with test_path_is_file

Shubham Verma· Mar 2, 2021, 18:50 UTC · lore

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(-)

Show changes to t/t9801-git-p4-branch.sh +19 −19

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 &&

-- 
2.25.1

Junio C Hamano· Mar 4, 2021, 01:11 UTC · re: Shubham Verma · lore

Re: [PATCH] t9801: replace test -f with test_path_is_file

Shubham Verma <shubhunic@gmail.com> writes:

Show 5 quoted lines

> -		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.

patcht9801: replace test -f with test_path_is_file