{"thread":{"id":"65181","subject":"[PATCH] t0004: replace test -e with test_path_exists","startedAt":"2026-03-09T17:36:47Z","lastAt":"2026-03-16T17:25:08Z","messageCount":5,"participants":["PRASHANT S BISHT","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"538303","messageId":"20260309173635.29683-1-prashantjee2025@gmail.com","threadId":"65181","inReplyTo":null,"subject":"[PATCH] t0004: replace test -e with test_path_exists","fromName":"PRASHANT S BISHT","fromEmail":"prashantjee2025@gmail.com","sentAt":"2026-03-09T17:36:35Z","receivedAt":"2026-03-09T17:36:47Z","isPatch":true,"sender":{"key":"prashantjee2025@gmail.com","avatar":"https://avatars.githubusercontent.com/u/213211543?v=4"},"body":"Replace old-style path existence checks with the modern test_path_exists\nhelper function that provides clearer diagnostic messages on failure.\nWhen test -e fails, the output gives no indication of what went wrong.\n\nThese instances were found using:\n\n  git grep \"test -[efd]\" t/ | grep -v \"if test\"\n\nas suggested in the microproject ideas.\n---\n t/t0004-unwritable.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh\nindex 3bdafbae0f..2a9fc781b6 100755\n--- a/t/t0004-unwritable.sh\n+++ b/t/t0004-unwritable.sh\n@@ -21,7 +21,7 @@ test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable reposi\n \ttest_must_fail git write-tree 2>out.write-tree\n '\n \n-test_lazy_prereq WRITE_TREE_OUT 'test -e \"$TRASH_DIRECTORY\"/out.write-tree'\n+test_lazy_prereq WRITE_TREE_OUT 'test_path_exists \"$TRASH_DIRECTORY/out.write-tree\"'\n test_expect_success WRITE_TREE_OUT 'write-tree output on unwritable repository' '\n \tcat >expect <<-\\EOF &&\n \terror: insufficient permission for adding an object to repository database .git/objects\n@@ -36,7 +36,7 @@ test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository\n \ttest_must_fail git commit -m second 2>out.commit\n '\n \n-test_lazy_prereq COMMIT_OUT 'test -e \"$TRASH_DIRECTORY\"/out.commit'\n+test_lazy_prereq COMMIT_OUT 'test_path_exists \"$TRASH_DIRECTORY/out.commit\"'\n test_expect_success COMMIT_OUT 'commit output on unwritable repository' '\n \tcat >expect <<-\\EOF &&\n \terror: insufficient permission for adding an object to repository database .git/objects\n@@ -52,7 +52,7 @@ test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repo\n \ttest_must_fail git update-index file 2>out.update-index\n '\n \n-test_lazy_prereq UPDATE_INDEX_OUT 'test -e \"$TRASH_DIRECTORY\"/out.update-index'\n+test_lazy_prereq UPDATE_INDEX_OUT 'test_path_exists \"$TRASH_DIRECTORY/out.update-index\"'\n test_expect_success UPDATE_INDEX_OUT 'update-index output on unwritable repository' '\n \tcat >expect <<-\\EOF &&\n \terror: insufficient permission for adding an object to repository database .git/objects\n@@ -69,7 +69,7 @@ test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '\n \ttest_must_fail git add file 2>out.add\n '\n \n-test_lazy_prereq ADD_OUT 'test -e \"$TRASH_DIRECTORY\"/out.add'\n+test_lazy_prereq ADD_OUT 'test_path_exists \"$TRASH_DIRECTORY/out.add\"'\n test_expect_success ADD_OUT 'add output on unwritable repository' '\n \tcat >expect <<-\\EOF &&\n \terror: insufficient permission for adding an object to repository database .git/objects\n-- \n2.50.1 (Apple Git-155)\n\n"},{"id":"538321","messageId":"xmqq4imo4sf1.fsf@gitster.g","threadId":"65181","inReplyTo":"20260309173635.29683-1-prashantjee2025@gmail.com","subject":"Re: [PATCH] t0004: replace test -e with test_path_exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T21:14:10Z","receivedAt":"2026-03-09T21:14:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"PRASHANT S BISHT <prashantjee2025@gmail.com> writes:\n\n> -test_lazy_prereq WRITE_TREE_OUT 'test -e \"$TRASH_DIRECTORY\"/out.write-tree'\n> +test_lazy_prereq WRITE_TREE_OUT 'test_path_exists \"$TRASH_DIRECTORY/out.write-tree\"'\n\nI suspect this is utterly wrong.  As you wrote in the proposed log\nmessage, test_path_exists is *NOT* about checking if the path\nexists.  It rather is about *expecting* for the path to exist, and\nfail *LOUDLY* if it does not.\n\nYou need to _think_ if we want a LOUD failure when somebody checks\nif a path exists and conditionally skip setting a test prerequisite\nwhen the path does not exist.  The original code is trying to be\nquiet, as the check is done not because existence of the checked\npath is good and lack of it is a test failure.  Lack of the path is\nexpected on places where the prerequisite is not set, and that by\nitself is not a test failure that you want a LOUD report about.\n\n"},{"id":"538329","messageId":"20260309224739.GA5682@coredump.intra.peff.net","threadId":"65181","inReplyTo":"xmqq4imo4sf1.fsf@gitster.g","subject":"Re: [PATCH] t0004: replace test -e with test_path_exists","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-09T22:47:39Z","receivedAt":"2026-03-09T22:47:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 09, 2026 at 02:14:10PM -0700, Junio C Hamano wrote:\n\n> PRASHANT S BISHT <prashantjee2025@gmail.com> writes:\n> \n> > -test_lazy_prereq WRITE_TREE_OUT 'test -e \"$TRASH_DIRECTORY\"/out.write-tree'\n> > +test_lazy_prereq WRITE_TREE_OUT 'test_path_exists \"$TRASH_DIRECTORY/out.write-tree\"'\n> \n> I suspect this is utterly wrong.  As you wrote in the proposed log\n> message, test_path_exists is *NOT* about checking if the path\n> exists.  It rather is about *expecting* for the path to exist, and\n> fail *LOUDLY* if it does not.\n> \n> You need to _think_ if we want a LOUD failure when somebody checks\n> if a path exists and conditionally skip setting a test prerequisite\n> when the path does not exist.  The original code is trying to be\n> quiet, as the check is done not because existence of the checked\n> path is good and lack of it is a test failure.  Lack of the path is\n> expected on places where the prerequisite is not set, and that by\n> itself is not a test failure that you want a LOUD report about.\n\nI'm not sure I agree. Verbose prereq blocks can help with debugging.\nNormally you would not see them at all, but if you are investigating why\na prereq did not trigger, you may want more output.\n\nWithout \"-v\" you would not see the output either way, like:\n\n  ok 1 # skip some test (missing FOO)\n\nBut with it, it is the difference between:\n\n  checking prerequisite: FOO\n  \n  mkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n  (\n  \tcd \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n  \ttest -e foo\n  \n  )\n  prerequisite FOO not satisfied\n  ok 1 # skip some test (missing FOO)\n\nand:\n\n  checking prerequisite: FOO\n  \n  mkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n  (\n  \tcd \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n  \ttest_path_exists foo\n  \n  )\n  Path foo doesn't exist\n  prerequisite FOO not satisfied\n  ok 1 # skip some test (missing FOO)\n\nProbably it's pretty obvious for a one-liner like this, but I think it\nwould help for a longer block.\n\n-Peff\n"},{"id":"538332","messageId":"xmqqpl5c1ttd.fsf@gitster.g","threadId":"65181","inReplyTo":"20260309224739.GA5682@coredump.intra.peff.net","subject":"Re: [PATCH] t0004: replace test -e with test_path_exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T23:12:14Z","receivedAt":"2026-03-09T23:12:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Without \"-v\" you would not see the output either way, like:\n>\n>   ok 1 # skip some test (missing FOO)\n>\n> But with it, it is the difference between:\n>\n>   checking prerequisite: FOO\n>   \n>   mkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n>   (\n>   \tcd \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n>   \ttest -e foo\n>   \n>   )\n>   prerequisite FOO not satisfied\n>   ok 1 # skip some test (missing FOO)\n>\n> and:\n>\n>   checking prerequisite: FOO\n>   \n>   mkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n>   (\n>   \tcd \"$TRASH_DIRECTORY/prereq-test-dir-FOO\" &&\n>   \ttest_path_exists foo\n>   \n>   )\n>   Path foo doesn't exist\n>   prerequisite FOO not satisfied\n>   ok 1 # skip some test (missing FOO)\n\nSorry, but I am not convinced.\n\nIt is as if satisfying FOO is the norm, and not satisifying FOO,\ni.e., missing path \"foo\", is something worth reporting about.\n\nIf the test reported both success and failure loudly, it may be a\ndifferent story, though.\n\n> Probably it's pretty obvious for a one-liner like this, but I think it\n> would help for a longer block.\n>\n> -Peff\n"},{"id":"539140","messageId":"20260316172457.38242-1-prashantjee2025@gmail.com","threadId":"65181","inReplyTo":"20260309173635.29683-1-prashantjee2025@gmail.com","subject":"[PATCH v2] t4200: convert test -[df] checks to test_path_* helpers","fromName":"PRASHANT S BISHT","fromEmail":"prashantjee2025@gmail.com","sentAt":"2026-03-16T17:24:57Z","receivedAt":"2026-03-16T17:25:08Z","isPatch":true,"sender":{"key":"prashantjee2025@gmail.com","avatar":"https://avatars.githubusercontent.com/u/213211543?v=4"},"body":"Replace old-style path existence checks in t4200-rerere.sh with\nthe appropriate test_path_* helper functions. These helpers provide\nclearer diagnostic messages on failure than the raw shell test\nbuiltin.\n\nSigned-off-by: Prashant S Bisht <prashantjee2025@gmail.com>\n---\n t/t4200-rerere.sh | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex 204325f4d5..1717f407c8 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -72,7 +72,7 @@ test_expect_success 'nothing recorded without rerere' '\n \trm -rf .git/rr-cache &&\n \tgit config rerere.enabled false &&\n \ttest_must_fail git merge first &&\n-\t! test -d .git/rr-cache\n+\ttest_path_is_missing .git/rr-cache\n '\n \n test_expect_success 'activate rerere, old style (conflicting merge)' '\n@@ -84,8 +84,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '\n \tsha1=$(sed \"s/\t.*//\" .git/MERGE_RR) &&\n \trr=.git/rr-cache/$sha1 &&\n \tgrep \"^=======\\$\" $rr/preimage &&\n-\t! test -f $rr/postimage &&\n-\t! test -f $rr/thisimage\n+\ttest_path_is_missing $rr/postimage &&\n+\ttest_path_is_missing $rr/thisimage\n '\n \n test_expect_success 'rerere.enabled works, too' '\n@@ -110,8 +110,8 @@ test_expect_success 'set up rr-cache' '\n \n test_expect_success 'rr-cache looks sane' '\n \t# no postimage or thisimage yet\n-\t! test -f $rr/postimage &&\n-\t! test -f $rr/thisimage &&\n+\ttest_path_is_missing $rr/postimage &&\n+\ttest_path_is_missing $rr/thisimage &&\n \n \t# preimage has right number of lines\n \tcnt=$(sed -ne \"/^<<<<<<</,/^>>>>>>>/p\" $rr/preimage | wc -l) &&\n@@ -167,7 +167,7 @@ test_expect_success 'first postimage wins' '\n \tgit show first:a1 | sed \"s/To die: t/To die! T/\" >expect &&\n \n \tgit commit -q -a -m \"prefer first over second\" &&\n-\ttest -f $rr/postimage &&\n+\ttest_path_is_file $rr/postimage &&\n \n \toldmtimepost=$(test-tool chmtime --get -60 $rr/postimage) &&\n \n@@ -190,14 +190,14 @@ test_expect_success 'rerere clear' '\n \tmv $rr/postimage .git/post-saved &&\n \techo \"$sha1\ta1\" | tr \"\\012\" \"\\000\" >.git/MERGE_RR &&\n \tgit rerere clear &&\n-\t! test -d $rr\n+\ttest_path_is_missing $rr\n '\n \n test_expect_success 'leftover directory' '\n \tgit reset --hard &&\n \tmkdir -p $rr &&\n \ttest_must_fail git merge first &&\n-\ttest -f $rr/preimage\n+\ttest_path_is_file $rr/preimage\n '\n \n test_expect_success 'missing preimage' '\n@@ -205,7 +205,7 @@ test_expect_success 'missing preimage' '\n \tmkdir -p $rr &&\n \tcp .git/post-saved $rr/postimage &&\n \ttest_must_fail git merge first &&\n-\ttest -f $rr/preimage\n+\ttest_path_is_file $rr/preimage\n '\n \n test_expect_success 'set up for garbage collection tests' '\n@@ -230,16 +230,16 @@ test_expect_success 'set up for garbage collection tests' '\n \n test_expect_success 'gc preserves young or recently used records' '\n \tgit rerere gc &&\n-\ttest -f $rr/preimage &&\n-\ttest -f $rr2/preimage\n+\ttest_path_is_file $rr/preimage &&\n+\ttest_path_is_file $rr2/preimage\n '\n \n test_expect_success 'old records rest in peace' '\n \ttest-tool chmtime =$just_over_60_days_ago $rr/postimage &&\n \ttest-tool chmtime =$just_over_15_days_ago $rr2/preimage &&\n \tgit rerere gc &&\n-\t! test -f $rr/preimage &&\n-\t! test -f $rr2/preimage\n+\ttest_path_is_missing $rr/preimage &&\n+\ttest_path_is_missing $rr2/preimage\n '\n \n rerere_gc_custom_expiry_test () {\n-- \n2.50.1 (Apple Git-155)\n\n"}]}