{"thread":{"id":"64959","subject":"[GSOC PATCH] t7003: modernize path existence checks using test helpers","startedAt":"2026-02-09T17:24:54Z","lastAt":"2026-02-10T18:42:04Z","messageCount":4,"participants":["SoutrikDas","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"535572","messageId":"20260209172445.39536-1-valusoutrik@gmail.com","threadId":"64959","inReplyTo":null,"subject":"[GSOC PATCH] t7003: modernize path existence checks using test helpers","fromName":"SoutrikDas","fromEmail":"valusoutrik@gmail.com","sentAt":"2026-02-09T17:24:45Z","receivedAt":"2026-02-09T17:24:54Z","isPatch":true,"sender":{"key":"valusoutrik@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56778179?v=4"},"body":"Replace direct uses of 'test -f' and 'test -d' with\ngit's helper functions 'test_path_is_file' ,\n'test_path_is_missing' and 'test_path_is_dir'\n\nSigned-off-by: SoutrikDas <valusoutrik@gmail.com>\n---\n t/t7003-filter-branch.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 5ab4d41ee7..c475769858 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -92,8 +92,8 @@ test_expect_success 'rewrite, renaming a specific file' '\n \n test_expect_success 'test that the file was renamed' '\n \ttest D = \"$(git show HEAD:doh --)\" &&\n-\t! test -f D.t &&\n-\ttest -f doh &&\n+\ttest_path_is_missing D.t &&\n+\ttest_path_is_file doh &&\n \ttest D = \"$(cat doh)\"\n '\n \n@@ -103,10 +103,10 @@ test_expect_success 'rewrite, renaming a specific directory' '\n \n test_expect_success 'test that the directory was renamed' '\n \ttest dir/D = \"$(git show HEAD:diroh/D.t --)\" &&\n-\t! test -d dir &&\n-\ttest -d diroh &&\n-\t! test -d diroh/dir &&\n-\ttest -f diroh/D.t &&\n+\ttest_path_is_missing dir &&\n+\ttest_path_is_dir diroh &&\n+\ttest_path_is_missing diroh/dir &&\n+\ttest_path_is_file diroh/D.t &&\n \ttest dir/D = \"$(cat diroh/D.t)\"\n '\n \n-- \n2.52.0\n\n"},{"id":"535585","messageId":"xmqqpl6d4wjh.fsf@gitster.g","threadId":"64959","inReplyTo":"20260209172445.39536-1-valusoutrik@gmail.com","subject":"Re: [GSOC PATCH] t7003: modernize path existence checks using test helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-09T18:11:14Z","receivedAt":"2026-02-09T18:11:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SoutrikDas <valusoutrik@gmail.com> writes:\n\n> Replace direct uses of 'test -f' and 'test -d' with\n> git's helper functions 'test_path_is_file' ,\n> 'test_path_is_missing' and 'test_path_is_dir'\n>\n> Signed-off-by: SoutrikDas <valusoutrik@gmail.com>\n> ---\n>  t/t7003-filter-branch.sh | 12 ++++++------\n>  1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n> index 5ab4d41ee7..c475769858 100755\n> --- a/t/t7003-filter-branch.sh\n> +++ b/t/t7003-filter-branch.sh\n> @@ -92,8 +92,8 @@ test_expect_success 'rewrite, renaming a specific file' '\n>  \n>  test_expect_success 'test that the file was renamed' '\n>  \ttest D = \"$(git show HEAD:doh --)\" &&\n> -\t! test -f D.t &&\n> -\ttest -f doh &&\n> +\ttest_path_is_missing D.t &&\n> +\ttest_path_is_file doh &&\n>  \ttest D = \"$(cat doh)\"\n>  '\n> @@ -103,10 +103,10 @@ test_expect_success 'rewrite, renaming a specific directory' '\n>  \n>  test_expect_success 'test that the directory was renamed' '\n>  \ttest dir/D = \"$(git show HEAD:diroh/D.t --)\" &&\n> -\t! test -d dir &&\n> -\ttest -d diroh &&\n> -\t! test -d diroh/dir &&\n> -\ttest -f diroh/D.t &&\n> +\ttest_path_is_missing dir &&\n> +\ttest_path_is_dir diroh &&\n> +\ttest_path_is_missing diroh/dir &&\n> +\ttest_path_is_file diroh/D.t &&\n>  \ttest dir/D = \"$(cat diroh/D.t)\"\n>  '\n\nAll the checks involving \"is-missing\" are now stricter than the\noriginal, in that they used to allow \"dir\" to exist as long as it is\nnot a directory, etc., but if we audited the code that leads to\nthese tests can never create a \"dir\" that is a regular file or\nsomething that is not a directory (which *I* did *NOT*, but\npresumably you have already done so?---if so that is worth noting in\nthe proposed log message), then \"test ! -d dir\" that is rewritten to\n\"test_path_is_missing dir\" is actually a _better_ test.\n\nThanks.\n\n\n"},{"id":"535694","messageId":"20260210181445.49380-1-valusoutrik@gmail.com","threadId":"64959","inReplyTo":"xmqqpl6d4wjh.fsf@gitster.g","subject":"Re: [GSOC PATCH] t7003: modernize path existence checks using test helpers","fromName":"SoutrikDas","fromEmail":"valusoutrik@gmail.com","sentAt":"2026-02-10T18:14:45Z","receivedAt":"2026-02-10T18:14:52Z","isPatch":true,"sender":{"key":"valusoutrik@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56778179?v=4"},"body":"> All the checks involving \"is-missing\" are now stricter than the\n> original, in that they used to allow \"dir\" to exist as long as it is\n> not a directory, etc., but if we audited the code that leads to\n> these tests can never create a \"dir\" that is a regular file or\n> something that is not a directory (which *I* did *NOT*, but\n> presumably you have already done so?\n\nAt the time of sending the patch v1, I did not do so. Sorry about that.\nNow I ran the test from start to 11, since test 12 was the one with two \nof those risky changes, ie `! test -d dir` and `! test -d diroh/dir`\n\nand after doing that I can confirm that there is no non directory dir \npresent before test 12 starts. Neither is there a non directory dir \ninside `diroh`\n\nThis was the output of ls -la \n\ndrwxr-xr-x  14 soutrik  staff  448 10 Feb 23:16 .\ndrwxr-xr-x   4 soutrik  staff  128 10 Feb 23:14 ..\ndrwxr-xr-x@ 14 soutrik  staff  448 10 Feb 23:16 .git\n-rw-r--r--   1 soutrik  staff    2 10 Feb 23:15 A.t\n-rw-r--r--   1 soutrik  staff    2 10 Feb 23:15 B.t\n-rw-r--r--@  1 soutrik  staff  128 10 Feb 23:16 backup-refs\n-rw-r--r--@  1 soutrik  staff    2 10 Feb 23:15 C.t\ndrwxr-xr-x@  3 soutrik  staff   96 10 Feb 23:16 diroh\n-rw-r--r--@  1 soutrik  staff    2 10 Feb 23:16 doh\ndrwxr-xr-x   4 soutrik  staff  128 10 Feb 23:15 drepo\ndrwxr-xr-x@  6 soutrik  staff  192 10 Feb 23:16 drepo-tree\n-rw-r--r--@  1 soutrik  staff    2 10 Feb 23:15 E.t\n-rw-r--r--   1 soutrik  staff    2 10 Feb 23:15 G.t\n-rw-r--r--   1 soutrik  staff    2 10 Feb 23:15 H.t\n\nAnd this was the ls -la in the `diroh` directory\n\ndrwxr-xr-x@  3 soutrik  staff   96 10 Feb 23:16 .\ndrwxr-xr-x  14 soutrik  staff  448 10 Feb 23:16 ..\n-rw-r--r--@  1 soutrik  staff    6 10 Feb 23:16 D.t\n\nOne thing is that I am not sure if what I did is ... the correct way\nto test this kind of thing... I just copy pasted all the test commands \ninto a big .sh file , did a git init -b main in a temp folder \nand ran the .sh file from there. And then observed the changes.\nThat big .sh file : https://pastebin.com/9QQD7qYA\n\n> ---if so that is worth noting in\n> the proposed log message), then \"test ! -d dir\" that is rewritten to\n> \"test_path_is_missing dir\" is actually a _better_ test.\n\nI am sending the v2 patch after this message. Should I have just put \nthis whole thing into the v2 patch cover mail ? \n\nBest,\nSoutrik\n"},{"id":"535702","messageId":"20260210184156.50363-1-valusoutrik@gmail.com","threadId":"64959","inReplyTo":"20260210181445.49380-1-valusoutrik@gmail.com","subject":"[GSOC PATCH v2] t7003: modernize path existence checks using test helpers","fromName":"SoutrikDas","fromEmail":"valusoutrik@gmail.com","sentAt":"2026-02-10T18:41:56Z","receivedAt":"2026-02-10T18:42:04Z","isPatch":true,"sender":{"key":"valusoutrik@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56778179?v=4"},"body":"Replace 'test -f' and 'test -d' with Git's path\nhelpers. Strengthen the '! test -d dir' and\n'! test -d diroh/dir' tests.\n\nChecking the test setup before test 12 confirms\nthat there are no expected non directories named\n'dir' or 'diroh/dir'\n\nSigned-off-by: SoutrikDas <valusoutrik@gmail.com>\n---\nv2:\nAddress feedback from Junio C Hamano \nAcknowledge that the rewritten tests are stricter\n---\n t/t7003-filter-branch.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 5ab4d41ee7..c475769858 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -92,8 +92,8 @@ test_expect_success 'rewrite, renaming a specific file' '\n \n test_expect_success 'test that the file was renamed' '\n \ttest D = \"$(git show HEAD:doh --)\" &&\n-\t! test -f D.t &&\n-\ttest -f doh &&\n+\ttest_path_is_missing D.t &&\n+\ttest_path_is_file doh &&\n \ttest D = \"$(cat doh)\"\n '\n \n@@ -103,10 +103,10 @@ test_expect_success 'rewrite, renaming a specific directory' '\n \n test_expect_success 'test that the directory was renamed' '\n \ttest dir/D = \"$(git show HEAD:diroh/D.t --)\" &&\n-\t! test -d dir &&\n-\ttest -d diroh &&\n-\t! test -d diroh/dir &&\n-\ttest -f diroh/D.t &&\n+\ttest_path_is_missing dir &&\n+\ttest_path_is_dir diroh &&\n+\ttest_path_is_missing diroh/dir &&\n+\ttest_path_is_file diroh/D.t &&\n \ttest dir/D = \"$(cat diroh/D.t)\"\n '\n \n-- \n2.52.0\n\n"}]}