{"thread":{"id":"60393","subject":"[Outreachy] [PATCH v2] t/t7601: use \"test_path_is_file\"etc. instead of \"test -f\"","startedAt":"2023-10-18T12:46:27Z","lastAt":"2023-10-18T23:59:34Z","messageCount":2,"participants":["Dorcas AnonoLitunya","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"483399","messageId":"20231018124538.68549-2-anonolitunya@gmail.com","threadId":"60393","inReplyTo":null,"subject":"[Outreachy] [PATCH v2] t/t7601: use \"test_path_is_file\"etc. instead of \"test -f\"","fromName":"Dorcas AnonoLitunya","fromEmail":"anonolitunya@gmail.com","sentAt":"2023-10-18T12:45:12Z","receivedAt":"2023-10-18T12:46:27Z","isPatch":true,"sender":{"key":"anonolitunya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/36160963?v=4"},"body":"Some tests in t7601 use \"test -f\" and \"test ! -f\" to see if a path\nexists or is missing.\n\nUse test_path_is_file and test_path_is_missing helper functions to\nclarify these tests a bit better. This especially matters for the\n\"missing\" case because \"test ! -f F\" will be happy if \"F\" exists as a\ndirectory, but the intent of the test is that \"F\" should not exist, even\nas a directory. The updated code expresses this better.\n\nSigned-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>\n---\n t/t7601-merge-pull-config.sh | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex bd238d89b0..e08767df66 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '\n \n test_expect_success 'merge c1 with c2' '\n \tgit reset --hard c1 &&\n-\ttest -f c0.c &&\n-\ttest -f c1.c &&\n-\ttest ! -f c2.c &&\n-\ttest ! -f c3.c &&\n+\ttest_path_is_file c0.c &&\n+\ttest_path_is_file c1.c &&\n+\ttest_path_is_missing c2.c &&\n+\ttest_path_is_missing c3.c &&\n \tgit merge c2 &&\n-\ttest -f c1.c &&\n-\ttest -f c2.c\n+\ttest_path_is_file c1.c &&\n+\ttest_path_is_file c2.c\n '\n \n test_expect_success 'fast-forward pull succeeds with \"true\" in pull.ff' '\n@@ -411,8 +411,8 @@ test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '\n \tgit reset --hard c1 &&\n \tgit config pull.twohead ours &&\n \tgit merge c2 &&\n-\ttest -f c1.c &&\n-\t! test -f c2.c\n+\ttest_path_is_file c1.c &&\n+\ttest_path_is_missing c2.c\n '\n \n test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '\n@@ -431,10 +431,10 @@ test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octo\n \ttest \"$(git rev-parse c2)\" = \"$(git rev-parse HEAD^2)\" &&\n \ttest \"$(git rev-parse c3)\" = \"$(git rev-parse HEAD^3)\" &&\n \tgit diff --exit-code &&\n-\ttest -f c0.c &&\n-\ttest -f c1.c &&\n-\ttest -f c2.c &&\n-\ttest -f c3.c\n+\ttest_path_is_file c0.c &&\n+\ttest_path_is_file c1.c &&\n+\ttest_path_is_file c2.c &&\n+\ttest_path_is_file c3.c\n '\n \n conflict_count()\n-- \n2.42.0.345.gaab89be2eb\n\n"},{"id":"483466","messageId":"xmqqa5sfzdlp.fsf@gitster.g","threadId":"60393","inReplyTo":"20231018124538.68549-2-anonolitunya@gmail.com","subject":"Re: [Outreachy] [PATCH v2] t/t7601: use \"test_path_is_file\"etc. instead of \"test -f\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-18T23:59:30Z","receivedAt":"2023-10-18T23:59:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dorcas AnonoLitunya <anonolitunya@gmail.com> writes:\n\n> Some tests in t7601 use \"test -f\" and \"test ! -f\" to see if a path\n> exists or is missing.\n>\n> Use test_path_is_file and test_path_is_missing helper functions to\n> clarify these tests a bit better. This especially matters for the\n> \"missing\" case because \"test ! -f F\" will be happy if \"F\" exists as a\n> directory, but the intent of the test is that \"F\" should not exist, even\n> as a directory. The updated code expresses this better.\n>\n> Signed-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>\n> ---\n>  t/t7601-merge-pull-config.sh | 24 ++++++++++++------------\n>  1 file changed, 12 insertions(+), 12 deletions(-)\n\nNicely done; it seems that the title line got garbled whitespace for\nsome reason but I'll fix them up while queuing.\n\nThanks.  Will queue.\n\n\n> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\n> index bd238d89b0..e08767df66 100755\n> --- a/t/t7601-merge-pull-config.sh\n> +++ b/t/t7601-merge-pull-config.sh\n> @@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '\n>  \n>  test_expect_success 'merge c1 with c2' '\n>  \tgit reset --hard c1 &&\n> -\ttest -f c0.c &&\n> -\ttest -f c1.c &&\n> -\ttest ! -f c2.c &&\n> -\ttest ! -f c3.c &&\n> +\ttest_path_is_file c0.c &&\n> +\ttest_path_is_file c1.c &&\n> +\ttest_path_is_missing c2.c &&\n> +\ttest_path_is_missing c3.c &&\n>  \tgit merge c2 &&\n> -\ttest -f c1.c &&\n> -\ttest -f c2.c\n> +\ttest_path_is_file c1.c &&\n> +\ttest_path_is_file c2.c\n>  '\n>  \n>  test_expect_success 'fast-forward pull succeeds with \"true\" in pull.ff' '\n> @@ -411,8 +411,8 @@ test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '\n>  \tgit reset --hard c1 &&\n>  \tgit config pull.twohead ours &&\n>  \tgit merge c2 &&\n> -\ttest -f c1.c &&\n> -\t! test -f c2.c\n> +\ttest_path_is_file c1.c &&\n> +\ttest_path_is_missing c2.c\n>  '\n>  \n>  test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '\n> @@ -431,10 +431,10 @@ test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octo\n>  \ttest \"$(git rev-parse c2)\" = \"$(git rev-parse HEAD^2)\" &&\n>  \ttest \"$(git rev-parse c3)\" = \"$(git rev-parse HEAD^3)\" &&\n>  \tgit diff --exit-code &&\n> -\ttest -f c0.c &&\n> -\ttest -f c1.c &&\n> -\ttest -f c2.c &&\n> -\ttest -f c3.c\n> +\ttest_path_is_file c0.c &&\n> +\ttest_path_is_file c1.c &&\n> +\ttest_path_is_file c2.c &&\n> +\ttest_path_is_file c3.c\n>  '\n>  \n>  conflict_count()\n\n"}]}