{"thread":{"id":"63048","subject":"[PATCH] [GSOC][PATCH] Modernize Test Path Checking in Git’s Test Suite","startedAt":"2025-03-03T14:18:32Z","lastAt":"2025-03-03T20:57:38Z","messageCount":3,"participants":["Prachit Ingle","Mahendra Dani","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"513403","messageId":"20250303141800.12848-1-ingleprachit101@gmail.com","threadId":"63048","inReplyTo":null,"subject":"[PATCH] [GSOC][PATCH] Modernize Test Path Checking in Git’s Test Suite","fromName":"Prachit Ingle","fromEmail":"ingleprachit101@gmail.com","sentAt":"2025-03-03T14:18:00Z","receivedAt":"2025-03-03T14:18:32Z","isPatch":true,"sender":{"key":"ingleprachit101@gmail.com","avatar":null},"body":"This patch improves the Git test suite by converting old-style path checks to use\nmodern Git test helpers. Specifically, we have replaced shell commands like `test -f`\nand `test -d` with the appropriate Git test helpers, such as `test_path_is_file` and\n`test_path_is_dir`. This enhances the readability and consistency of the test suite.\n\nThe following tests were updated:\n- t/chainlint/cuddled-loop.test\n- t/chainlint/cuddled.test\n- t/chainlint/double-here-doc.test\n- t/chainlint/dqstring-line-splice.test\n- t/chainlint/dqstring-no-interpolate.test\n- t/chainlint/empty-here-doc.test\n- t/chainlint/exclamation.test\n- t/chainlint/exit-loop.test\n- t/chainlint/exit-subshell.test\n- t/chainlint/for-loop-abbreviated.test\n- t/chainlint/for-loop.test\n- t/chainlint/function.expect\n- t/chainlint/function.test\n- t/chainlint/here-doc-body-indent.test\n\nThe changes have been verified by running the test suite to ensure no breaks or regressions.\n\nCommand used to find instances: git grep 'test -[efd]' t/\n\nSigned-off-by: Prachit Ingle <ingleprachit101@gmail.com>\n---\n t/chainlint/chained-subshell.expect |  2 +-\n t/chainlint/chained-subshell.test   | 30 ++++++++++++++---------------\n t/chainlint/function.expect         |  2 +-\n t/chainlint/function.test           | 30 ++++++++++++++---------------\n t/interop/interop-lib.sh            |  4 ++--\n t/lib-httpd/apply-one-time-perl.sh  |  2 +-\n t/lib-httpd/nph-custom-auth.sh      |  2 +-\n t/perf/p5302-pack-index.sh          |  2 +-\n t/perf/p7527-builtin-fsmonitor.sh   |  2 +-\n t/perf/perf-lib.sh                  |  2 +-\n 10 files changed, 39 insertions(+), 39 deletions(-)\n\ndiff --git a/t/chainlint/chained-subshell.expect b/t/chainlint/chained-subshell.expect\nindex 93fb1a6578..49efa1301d 100644\n--- a/t/chainlint/chained-subshell.expect\n+++ b/t/chainlint/chained-subshell.expect\n@@ -5,6 +5,6 @@\n 6 ) &&\n 7 \n 8 cut \"-d \" -f actual | (read s1 s2 s3 &&\n-9 test -f $s1 ?!LINT: missing '&&'?!\n+9 test_path_is_file \"$s1\" || error \"$s1 should exist\"\n 10 test $(cat $s2) = tree2path1 &&\n 11 test $(cat $s3) = tree3path1)\ndiff --git a/t/chainlint/chained-subshell.test b/t/chainlint/chained-subshell.test\nindex 1f11f65398..e167335f1e 100644\n--- a/t/chainlint/chained-subshell.test\n+++ b/t/chainlint/chained-subshell.test\n@@ -1,15 +1,15 @@\n-test_expect_success 'chained-subshell' '\n-# LINT: start of subshell chained to preceding command\n-mkdir sub && (\n-\tcd sub &&\n-\tfoo the bar\n-\tnuff said\n-) &&\n-\n-# LINT: preceding command pipes to subshell on same line\n-cut \"-d \" -f actual | (read s1 s2 s3 &&\n-test -f $s1\n-test $(cat $s2) = tree2path1 &&\n-# LINT: closing subshell \")\" correctly detected on same line as \"$(...)\"\n-test $(cat $s3) = tree3path1)\n-'\n+test_expect_success 'chained-subshell' '\n+# LINT: start of subshell chained to preceding command\n+mkdir sub && (\n+\tcd sub &&\n+\tfoo the bar\n+\tnuff said\n+) &&\n+\n+# LINT: preceding command pipes to subshell on same line\n+cut \"-d \" -f actual | (read s1 s2 s3 &&\n+test_path_is_file $s1\n+test $(cat $s2) = tree2path1 &&\n+# LINT: closing subshell \")\" correctly detected on same line as \"$(...)\"\n+test $(cat $s3) = tree3path1)\n+'\ndiff --git a/t/chainlint/function.expect b/t/chainlint/function.expect\nindex 9e46a3554a..a0a465e6d4 100644\n--- a/t/chainlint/function.expect\n+++ b/t/chainlint/function.expect\n@@ -4,7 +4,7 @@\n 5 \n 6 remove_object() {\n 7 \tfile=$(sha1_file \"$*\") &&\n-8 \ttest -e \"$file\" ?!LINT: missing '&&'?!\n+8 \ttest_path_exists \"$file\" || error \"$file should exist\"\n 9 \trm -f \"$file\"\n 10 } ?!LINT: missing '&&'?!\n 11 \ndiff --git a/t/chainlint/function.test b/t/chainlint/function.test\nindex 763fcf3f87..841c60720c 100644\n--- a/t/chainlint/function.test\n+++ b/t/chainlint/function.test\n@@ -1,15 +1,15 @@\n-test_expect_success 'function' '\n-# LINT: \"()\" in function definition not mistaken for subshell\n-sha1_file() {\n-\techo \"$*\" | sed \"s#..#.git/objects/&/#\"\n-} &&\n-\n-# LINT: broken &&-chain in function and after function\n-remove_object() {\n-\tfile=$(sha1_file \"$*\") &&\n-\ttest -e \"$file\"\n-\trm -f \"$file\"\n-}\n-\n-sha1_file arg && remove_object arg\n-'\n+test_expect_success 'function' '\n+# LINT: \"()\" in function definition not mistaken for subshell\n+sha1_file() {\n+\techo \"$*\" | sed \"s#..#.git/objects/&/#\"\n+} &&\n+\n+# LINT: broken &&-chain in function and after function\n+remove_object() {\n+\tfile=$(sha1_file \"$*\") &&\n+\ttest_path_exists \"$file\" || error \"$file should exist\"\n+\trm -f \"$file\"\n+}\n+\n+sha1_file arg && remove_object arg\n+'\ndiff --git a/t/interop/interop-lib.sh b/t/interop/interop-lib.sh\nindex 1b5864d2a7..b2055d4252 100644\n--- a/t/interop/interop-lib.sh\n+++ b/t/interop/interop-lib.sh\n@@ -21,7 +21,7 @@ build_version () {\n \tsha1=$(git rev-parse \"$1^{tree}\") || return 1\n \tdir=$BUILD_ROOT/$sha1\n \n-\tif test -e \"$dir/.built\"\n+\tif test_path_exists \"$dir/.built\"\n \tthen\n \t\techo \"$dir\"\n \t\treturn 0\n@@ -37,7 +37,7 @@ build_version () {\n \n \tfor config in config.mak config.mak.autogen config.status\n \tdo\n-\t\tif test -e \"$INTEROP_ROOT/../../$config\"\n+\t\tif test_path_exists \"$INTEROP_ROOT/../../$config\"\n \t\tthen\n \t\t\tcp \"$INTEROP_ROOT/../../$config\" \"$dir/\" || return 1\n \t\tfi\ndiff --git a/t/lib-httpd/apply-one-time-perl.sh b/t/lib-httpd/apply-one-time-perl.sh\nindex d7f9fed6ae..83ede36efb 100644\n--- a/t/lib-httpd/apply-one-time-perl.sh\n+++ b/t/lib-httpd/apply-one-time-perl.sh\n@@ -7,7 +7,7 @@\n #\n # This can be used to simulate the effects of the repository changing in\n # between HTTP request-response pairs.\n-if test -f one-time-perl\n+if test_path_is_file one-time-perl\n then\n \tLC_ALL=C\n \texport LC_ALL\ndiff --git a/t/lib-httpd/nph-custom-auth.sh b/t/lib-httpd/nph-custom-auth.sh\nindex d408d2caad..c39b816c45 100644\n--- a/t/lib-httpd/nph-custom-auth.sh\n+++ b/t/lib-httpd/nph-custom-auth.sh\n@@ -41,7 +41,7 @@ then\n fi\n \n echo 'HTTP/1.1 401 Authorization Required'\n-if test -f \"$CHALLENGE_FILE\"\n+if test_path_is_file \"$CHALLENGE_FILE\"\n then\n \tsed -ne 's/^id=default.*response=//p' \"$CHALLENGE_FILE\"\n fi\ndiff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh\nindex 14c601bbf8..d3a3ea360f 100755\n--- a/t/perf/p5302-pack-index.sh\n+++ b/t/perf/p5302-pack-index.sh\n@@ -9,7 +9,7 @@ test_perf_large_repo\n test_expect_success 'repack' '\n \tgit repack -ad &&\n \tPACK=$(ls .git/objects/pack/*.pack | head -n1) &&\n-\ttest -f \"$PACK\" &&\n+\ttest_path_is_file \"$PACK\" &&\n \texport PACK\n '\n \ndiff --git a/t/perf/p7527-builtin-fsmonitor.sh b/t/perf/p7527-builtin-fsmonitor.sh\nindex 90164327e8..16e56cc197 100755\n--- a/t/perf/p7527-builtin-fsmonitor.sh\n+++ b/t/perf/p7527-builtin-fsmonitor.sh\n@@ -46,7 +46,7 @@ export TMP_BR\n REPO=../repos/gen-many-files-\"$PARAMS\".git\n export REPO\n \n-if ! test -d $REPO\n+if ! test_path_is_dir $REPO\n then\n \t(cd ../repos; ./many-files.sh -d $PARAM_D -w $PARAM_W -f $PARAM_F)\n fi\ndiff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\nindex 8ab6d9c469..13eb33ae6f 100644\n--- a/t/perf/perf-lib.sh\n+++ b/t/perf/perf-lib.sh\n@@ -117,7 +117,7 @@ test_perf_create_repo_from () {\n \t\t\"$MODERN_GIT\" init -q &&\n \t\ttest_perf_do_repo_symlink_config_ &&\n \t\tmv .git/hooks .git/hooks-disabled 2>/dev/null &&\n-\t\tif test -f .git/index.lock\n+\t\tif test_path_is_file .git/index.lock\n \t\tthen\n \t\t\t# We may be copying a repo that can't run \"git\n \t\t\t# status\" due to a locked index. Since we have\n-- \n2.34.1\n\n"},{"id":"513427","messageId":"CABGrwBAhtf0Ua=0i4N7XV-K7d8iphLQfXczPnN+kLRRNE-QxxQ@mail.gmail.com","threadId":"63048","inReplyTo":"20250303141800.12848-1-ingleprachit101@gmail.com","subject":"Re: [PATCH] [GSOC][PATCH] Modernize Test Path Checking in Git’s Test Suite","fromName":"Mahendra Dani","fromEmail":"danimahendra0904@gmail.com","sentAt":"2025-03-03T20:04:21Z","receivedAt":"2025-03-03T20:04:35Z","isPatch":true,"sender":{"key":"danimahendra0904@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116940083?v=4"},"body":"On Mon, Mar 3, 2025 at 7:49 PM Prachit Ingle <ingleprachit101@gmail.com> wrote:\n>\n> This patch improves the Git test suite by converting old-style path checks to use\n> modern Git test helpers. Specifically, we have replaced shell commands like `test -f`\n> and `test -d` with the appropriate Git test helpers, such as `test_path_is_file` and\n> `test_path_is_dir`. This enhances the readability and consistency of the test suite.\n>\n> The following tests were updated:\n> - t/chainlint/cuddled-loop.test\n> - t/chainlint/cuddled.test\n> - t/chainlint/double-here-doc.test\n> - t/chainlint/dqstring-line-splice.test\n> - t/chainlint/dqstring-no-interpolate.test\n> - t/chainlint/empty-here-doc.test\n> - t/chainlint/exclamation.test\n> - t/chainlint/exit-loop.test\n> - t/chainlint/exit-subshell.test\n> - t/chainlint/for-loop-abbreviated.test\n> - t/chainlint/for-loop.test\n> - t/chainlint/function.expect\n> - t/chainlint/function.test\n> - t/chainlint/here-doc-body-indent.test\n>\n> The changes have been verified by running the test suite to ensure no breaks or regressions.\n>\n> Command used to find instances: git grep 'test -[efd]' t/\n>\n> Signed-off-by: Prachit Ingle <ingleprachit101@gmail.com>\n> ---\n>  t/chainlint/chained-subshell.expect |  2 +-\n>  t/chainlint/chained-subshell.test   | 30 ++++++++++++++---------------\n>  t/chainlint/function.expect         |  2 +-\n>  t/chainlint/function.test           | 30 ++++++++++++++---------------\n>  t/interop/interop-lib.sh            |  4 ++--\n>  t/lib-httpd/apply-one-time-perl.sh  |  2 +-\n>  t/lib-httpd/nph-custom-auth.sh      |  2 +-\n>  t/perf/p5302-pack-index.sh          |  2 +-\n>  t/perf/p7527-builtin-fsmonitor.sh   |  2 +-\n>  t/perf/perf-lib.sh                  |  2 +-\n>  10 files changed, 39 insertions(+), 39 deletions(-)\n>\n> diff --git a/t/chainlint/chained-subshell.expect b/t/chainlint/chained-subshell.expect\n> index 93fb1a6578..49efa1301d 100644\n> --- a/t/chainlint/chained-subshell.expect\n> +++ b/t/chainlint/chained-subshell.expect\n> @@ -5,6 +5,6 @@\n>  6 ) &&\n>  7\n>  8 cut \"-d \" -f actual | (read s1 s2 s3 &&\n> -9 test -f $s1 ?!LINT: missing '&&'?!\n> +9 test_path_is_file \"$s1\" || error \"$s1 should exist\"\n>  10 test $(cat $s2) = tree2path1 &&\n>  11 test $(cat $s3) = tree3path1)\n> diff --git a/t/chainlint/chained-subshell.test b/t/chainlint/chained-subshell.test\n> index 1f11f65398..e167335f1e 100644\n> --- a/t/chainlint/chained-subshell.test\n> +++ b/t/chainlint/chained-subshell.test\n> @@ -1,15 +1,15 @@\n> -test_expect_success 'chained-subshell' '\n> -# LINT: start of subshell chained to preceding command\n> -mkdir sub && (\n> -       cd sub &&\n> -       foo the bar\n> -       nuff said\n> -) &&\n> -\n> -# LINT: preceding command pipes to subshell on same line\n> -cut \"-d \" -f actual | (read s1 s2 s3 &&\n> -test -f $s1\n> -test $(cat $s2) = tree2path1 &&\n> -# LINT: closing subshell \")\" correctly detected on same line as \"$(...)\"\n> -test $(cat $s3) = tree3path1)\n> -'\n> +test_expect_success 'chained-subshell' '\n> +# LINT: start of subshell chained to preceding command\n> +mkdir sub && (\n> +       cd sub &&\n> +       foo the bar\n> +       nuff said\n> +) &&\n> +\n> +# LINT: preceding command pipes to subshell on same line\n> +cut \"-d \" -f actual | (read s1 s2 s3 &&\n> +test_path_is_file $s1\n> +test $(cat $s2) = tree2path1 &&\n> +# LINT: closing subshell \")\" correctly detected on same line as \"$(...)\"\n> +test $(cat $s3) = tree3path1)\n> +'\n> diff --git a/t/chainlint/function.expect b/t/chainlint/function.expect\n> index 9e46a3554a..a0a465e6d4 100644\n> --- a/t/chainlint/function.expect\n> +++ b/t/chainlint/function.expect\n> @@ -4,7 +4,7 @@\n>  5\n>  6 remove_object() {\n>  7      file=$(sha1_file \"$*\") &&\n> -8      test -e \"$file\" ?!LINT: missing '&&'?!\n> +8      test_path_exists \"$file\" || error \"$file should exist\"\n>  9      rm -f \"$file\"\n>  10 } ?!LINT: missing '&&'?!\n>  11\n> diff --git a/t/chainlint/function.test b/t/chainlint/function.test\n> index 763fcf3f87..841c60720c 100644\n> --- a/t/chainlint/function.test\n> +++ b/t/chainlint/function.test\n> @@ -1,15 +1,15 @@\n> -test_expect_success 'function' '\n> -# LINT: \"()\" in function definition not mistaken for subshell\n> -sha1_file() {\n> -       echo \"$*\" | sed \"s#..#.git/objects/&/#\"\n> -} &&\n> -\n> -# LINT: broken &&-chain in function and after function\n> -remove_object() {\n> -       file=$(sha1_file \"$*\") &&\n> -       test -e \"$file\"\n> -       rm -f \"$file\"\n> -}\n> -\n> -sha1_file arg && remove_object arg\n> -'\n> +test_expect_success 'function' '\n> +# LINT: \"()\" in function definition not mistaken for subshell\n> +sha1_file() {\n> +       echo \"$*\" | sed \"s#..#.git/objects/&/#\"\n> +} &&\n> +\n> +# LINT: broken &&-chain in function and after function\n> +remove_object() {\n> +       file=$(sha1_file \"$*\") &&\n> +       test_path_exists \"$file\" || error \"$file should exist\"\n> +       rm -f \"$file\"\n> +}\n> +\n> +sha1_file arg && remove_object arg\n> +'\n> diff --git a/t/interop/interop-lib.sh b/t/interop/interop-lib.sh\n> index 1b5864d2a7..b2055d4252 100644\n> --- a/t/interop/interop-lib.sh\n> +++ b/t/interop/interop-lib.sh\n> @@ -21,7 +21,7 @@ build_version () {\n>         sha1=$(git rev-parse \"$1^{tree}\") || return 1\n>         dir=$BUILD_ROOT/$sha1\n>\n> -       if test -e \"$dir/.built\"\n> +       if test_path_exists \"$dir/.built\"\n>         then\n>                 echo \"$dir\"\n>                 return 0\n> @@ -37,7 +37,7 @@ build_version () {\n>\n>         for config in config.mak config.mak.autogen config.status\n>         do\n> -               if test -e \"$INTEROP_ROOT/../../$config\"\n> +               if test_path_exists \"$INTEROP_ROOT/../../$config\"\n>                 then\n>                         cp \"$INTEROP_ROOT/../../$config\" \"$dir/\" || return 1\n>                 fi\n> diff --git a/t/lib-httpd/apply-one-time-perl.sh b/t/lib-httpd/apply-one-time-perl.sh\n> index d7f9fed6ae..83ede36efb 100644\n> --- a/t/lib-httpd/apply-one-time-perl.sh\n> +++ b/t/lib-httpd/apply-one-time-perl.sh\n> @@ -7,7 +7,7 @@\n>  #\n>  # This can be used to simulate the effects of the repository changing in\n>  # between HTTP request-response pairs.\n> -if test -f one-time-perl\n> +if test_path_is_file one-time-perl\n>  then\n>         LC_ALL=C\n>         export LC_ALL\n> diff --git a/t/lib-httpd/nph-custom-auth.sh b/t/lib-httpd/nph-custom-auth.sh\n> index d408d2caad..c39b816c45 100644\n> --- a/t/lib-httpd/nph-custom-auth.sh\n> +++ b/t/lib-httpd/nph-custom-auth.sh\n> @@ -41,7 +41,7 @@ then\n>  fi\n>\n>  echo 'HTTP/1.1 401 Authorization Required'\n> -if test -f \"$CHALLENGE_FILE\"\n> +if test_path_is_file \"$CHALLENGE_FILE\"\n>  then\n>         sed -ne 's/^id=default.*response=//p' \"$CHALLENGE_FILE\"\n>  fi\n> diff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh\n> index 14c601bbf8..d3a3ea360f 100755\n> --- a/t/perf/p5302-pack-index.sh\n> +++ b/t/perf/p5302-pack-index.sh\n> @@ -9,7 +9,7 @@ test_perf_large_repo\n>  test_expect_success 'repack' '\n>         git repack -ad &&\n>         PACK=$(ls .git/objects/pack/*.pack | head -n1) &&\n> -       test -f \"$PACK\" &&\n> +       test_path_is_file \"$PACK\" &&\n>         export PACK\n>  '\n>\n> diff --git a/t/perf/p7527-builtin-fsmonitor.sh b/t/perf/p7527-builtin-fsmonitor.sh\n> index 90164327e8..16e56cc197 100755\n> --- a/t/perf/p7527-builtin-fsmonitor.sh\n> +++ b/t/perf/p7527-builtin-fsmonitor.sh\n> @@ -46,7 +46,7 @@ export TMP_BR\n>  REPO=../repos/gen-many-files-\"$PARAMS\".git\n>  export REPO\n>\n> -if ! test -d $REPO\n> +if ! test_path_is_dir $REPO\n>  then\n>         (cd ../repos; ./many-files.sh -d $PARAM_D -w $PARAM_W -f $PARAM_F)\n>  fi\n> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\n> index 8ab6d9c469..13eb33ae6f 100644\n> --- a/t/perf/perf-lib.sh\n> +++ b/t/perf/perf-lib.sh\n> @@ -117,7 +117,7 @@ test_perf_create_repo_from () {\n>                 \"$MODERN_GIT\" init -q &&\n>                 test_perf_do_repo_symlink_config_ &&\n>                 mv .git/hooks .git/hooks-disabled 2>/dev/null &&\n> -               if test -f .git/index.lock\n> +               if test_path_is_file .git/index.lock\n>                 then\n>                         # We may be copying a repo that can't run \"git\n>                         # status\" due to a locked index. Since we have\n> --\n> 2.34.1\n>\n>\n\nThe `test_path_exsists()`, `test_path_is_file()` and\n`test_path_is_dir()` helper functions\nare expected to be used within `test_expect_success` (or\n`test_expect_failure`) blocks [1]. However, in several of the above\nscripts, this condition\nis not met, making them unsuitable candidates for such modifications.\n\nAdditionally, it is a good practice to make separate commits for\nlogically separate changes\nand send multiple patches in a series [2-3], i.e submit one patch per\ntest script.\n\nThanks,\nMahendra\n\n[References]\n1. https://lore.kernel.org/git/CAPig+cR2-6qONkosu7=qEQSJa_fvYuVQ0to47D5qx904zW08Eg@mail.gmail.com/\n2. https://git-scm.com/docs/MyFirstContribution\n3. https://git-scm.com/docs/SubmittingPatches\n"},{"id":"513470","messageId":"CAPig+cTJYsXTSXHyUEDcG+qxUNdh5aVUV7FwC=gHDbUX0mpZsA@mail.gmail.com","threadId":"63048","inReplyTo":"20250303141800.12848-1-ingleprachit101@gmail.com","subject":"Re: [PATCH] [GSOC][PATCH] Modernize Test Path Checking in Git’s Test Suite","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-03-03T20:57:25Z","receivedAt":"2025-03-03T20:57:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 3, 2025 at 9:20 AM Prachit Ingle <ingleprachit101@gmail.com> wrote:\n> This patch improves the Git test suite by converting old-style path checks to use\n> modern Git test helpers. Specifically, we have replaced shell commands like `test -f`\n> and `test -d` with the appropriate Git test helpers, such as `test_path_is_file` and\n> `test_path_is_dir`. This enhances the readability and consistency of the test suite.\n>\n> The following tests were updated:\n> - t/chainlint/cuddled-loop.test\n> - t/chainlint/cuddled.test\n> - t/chainlint/double-here-doc.test\n> - t/chainlint/dqstring-line-splice.test\n> - t/chainlint/dqstring-no-interpolate.test\n> - t/chainlint/empty-here-doc.test\n> - t/chainlint/exclamation.test\n> - t/chainlint/exit-loop.test\n> - t/chainlint/exit-subshell.test\n> - t/chainlint/for-loop-abbreviated.test\n> - t/chainlint/for-loop.test\n> - t/chainlint/function.expect\n> - t/chainlint/function.test\n> - t/chainlint/here-doc-body-indent.test\n\nThree comments:\n\n* no need to provide an itemized list of files you changed since\ngit-format-patch does it for you automatically (below the \"---\" line)\n\n* the list you presented above does not match the list automatically\ngenerated by git-format-patch\n\n* let's not touch any of the \"chainlint\" files; they are checking\nvalidity of a completely separate tool (\"chainlint\"), and have nothing\nto do with checking Git itself\n\n> The changes have been verified by running the test suite to ensure no breaks or regressions.\n>\n> Command used to find instances: git grep 'test -[efd]' t/\n>\n> Signed-off-by: Prachit Ingle <ingleprachit101@gmail.com>\n> ---\n>  t/chainlint/chained-subshell.expect |  2 +-\n>  t/chainlint/chained-subshell.test   | 30 ++++++++++++++---------------\n>  t/chainlint/function.expect         |  2 +-\n>  t/chainlint/function.test           | 30 ++++++++++++++---------------\n>  t/interop/interop-lib.sh            |  4 ++--\n>  t/lib-httpd/apply-one-time-perl.sh  |  2 +-\n>  t/lib-httpd/nph-custom-auth.sh      |  2 +-\n>  t/perf/p5302-pack-index.sh          |  2 +-\n>  t/perf/p7527-builtin-fsmonitor.sh   |  2 +-\n>  t/perf/perf-lib.sh                  |  2 +-\n>  10 files changed, 39 insertions(+), 39 deletions(-)\n>\n> diff --git a/t/interop/interop-lib.sh b/t/interop/interop-lib.sh\n> @@ -37,7 +37,7 @@ build_version () {\n>         for config in config.mak config.mak.autogen config.status\n>         do\n> -               if test -e \"$INTEROP_ROOT/../../$config\"\n> +               if test_path_exists \"$INTEROP_ROOT/../../$config\"\n>                 then\n>                         cp \"$INTEROP_ROOT/../../$config\" \"$dir/\" || return 1\n>                 fi\n\nUnfortunately, this and almost all other changes in this patch are\nincorrect. The `test_path_foo` functions are meant to be used only as\n*assertions*, not as part of general control flow. That is, they are\nmeant to be linked into a &&-chain within a `test_expect_success` body\nlike this:\n\n    some command &&\n    test_path_exists foo/bar &&\n    some other command &&\n    ...\n\nTo understand why, take a look at the definition of `test_path_exists `:\n\n    test_path_exists () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        if ! test -e \"$1\"\n        then\n            echo \"Path $1 doesn't exist\"\n            false\n        fi\n    }\n\nThis means that `test_path_exists` will print an error message when\nthe path doesn't exist, but that's inappropriate as a general control\nflow mechanism in which it may be perfectly legitimate for a path to\nbe missing.\n\n> diff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh\n> @@ -9,7 +9,7 @@ test_perf_large_repo\n>  test_expect_success 'repack' '\n>         git repack -ad &&\n>         PACK=$(ls .git/objects/pack/*.pack | head -n1) &&\n> -       test -f \"$PACK\" &&\n> +       test_path_is_file \"$PACK\" &&\n>         export PACK\n>  '\n\nHence, this is the only change in the entire patch which is likely correct.\n"}]}