{"thread":{"id":"55242","subject":"[PATCH] t9801: replace test -f with test_path_is_file","startedAt":"2021-03-03T06:41:56Z","lastAt":"2021-03-04T01:15:21Z","messageCount":2,"participants":["Shubham Verma","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"418118","messageId":"20210302185056.65929-1-shubhunic@gmail.com","threadId":"55242","inReplyTo":null,"subject":"[PATCH] t9801: replace test -f with test_path_is_file","fromName":"Shubham Verma","fromEmail":"shubhunic@gmail.com","sentAt":"2021-03-02T18:50:56Z","receivedAt":"2021-03-03T06:41:56Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"Although `test -f` has the same functionality as test_path_is_file(), in\nthe case where test_path_is_file() fails, we get much better debugging\ninformation.\n\nReplace `test -f` with test_path_is_file so that future developers\nwill have a better experience debugging these test cases.\n\nSigned-off-by: Shubham Verma <shubhunic@gmail.com>\n---\n t/t9801-git-p4-branch.sh | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 67ff2711f5..214c2a183d 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -200,19 +200,19 @@ test_expect_success 'git p4 clone simple branches' '\n \t\tgit p4 clone --dest=. --detect-branches //depot@all &&\n \t\tgit log --all --graph --decorate --stat &&\n \t\tgit reset --hard p4/depot/branch1 &&\n-\t\ttest -f file1 &&\n-\t\ttest -f file2 &&\n-\t\ttest -f file3 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_file file2 &&\n+\t\ttest_path_is_file file3 &&\n \t\tgrep update file2 &&\n \t\tgit reset --hard p4/depot/branch2 &&\n-\t\ttest -f file1 &&\n-\t\ttest -f file2 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_file file2 &&\n \t\ttest ! -f file3 &&\n \t\t! grep update file2 &&\n \t\tgit reset --hard p4/depot/branch3 &&\n-\t\ttest -f file1 &&\n-\t\ttest -f file2 &&\n-\t\ttest -f file3 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_file file2 &&\n+\t\ttest_path_is_file file3 &&\n \t\tgrep update file2 &&\n \t\tcd \"$cli\" &&\n \t\tcd branch1 &&\n@@ -603,22 +603,22 @@ test_expect_success 'git p4 clone simple branches with base folder on server sid\n \t\tgit p4 clone --dest=. --use-client-spec  --detect-branches //depot@all &&\n \t\tgit log --all --graph --decorate --stat &&\n \t\tgit reset --hard p4/depot/branch1 &&\n-\t\ttest -f file1 &&\n-\t\ttest -f file2 &&\n-\t\ttest -f file3 &&\n-\t\ttest -f sub_file1 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_file file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\ttest_path_is_file sub_file1 &&\n \t\tgrep update file2 &&\n \t\tgit reset --hard p4/depot/branch2 &&\n-\t\ttest -f file1 &&\n-\t\ttest -f file2 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_file file2 &&\n \t\ttest ! -f file3 &&\n-\t\ttest -f sub_file1 &&\n+\t\ttest_path_is_file sub_file1 &&\n \t\t! grep update file2 &&\n \t\tgit reset --hard p4/depot/branch3 &&\n-\t\ttest -f file1 &&\n-\t\ttest -f file2 &&\n-\t\ttest -f file3 &&\n-\t\ttest -f sub_file1 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_file file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\ttest_path_is_file sub_file1 &&\n \t\tgrep update file2 &&\n \t\tcd \"$cli\" &&\n \t\tcd branch1 &&\n-- \n2.25.1\n\n"},{"id":"418199","messageId":"xmqq8s73g1tr.fsf@gitster.c.googlers.com","threadId":"55242","inReplyTo":"20210302185056.65929-1-shubhunic@gmail.com","subject":"Re: [PATCH] t9801: replace test -f with test_path_is_file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-04T01:11:28Z","receivedAt":"2021-03-04T01:15:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shubham Verma <shubhunic@gmail.com> writes:\n\n> -\t\ttest -f file1 &&\n> -\t\ttest -f file2 &&\n> +\t\ttest_path_is_file file1 &&\n> +\t\ttest_path_is_file file2 &&\n>  \t\ttest ! -f file3 &&\n\nYou chose not to touch the last one, which is the right thing to do.\nReplacing \"test -f P\" with \"test_path_is_file P\" can be done\nmechanically, but it takes understanding of what the test is doing\nto come up the correct replacement for \"test ! -f P\".  It may be\nexpecting P to be a directory, or it may be expecting P to be\nmissing, for example.\n\nWill queue.  Thanks.\n\n"}]}