[PATCH] t0004: replace test -e with test_path_exists

[PATCH] t0004: replace test -e with test_path_exists

[PRASHANT S BISHT]

Replace old-style path existence checks with the modern test_path_exists
helper function that provides clearer diagnostic messages on failure.
When test -e fails, the output gives no indication of what went wrong.

These instances were found using:

  git grep "test -[efd]" t/ | grep -v "if test"

as suggested in the microproject ideas.
---
 t/t0004-unwritable.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
index 3bdafbae0f..2a9fc781b6 100755
--- a/t/t0004-unwritable.sh
+++ b/t/t0004-unwritable.sh
@@ -21,7 +21,7 @@ test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable reposi
 	test_must_fail git write-tree 2>out.write-tree
 '
 
-test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
+test_lazy_prereq WRITE_TREE_OUT 'test_path_exists "$TRASH_DIRECTORY/out.write-tree"'
 test_expect_success WRITE_TREE_OUT 'write-tree output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
@@ -36,7 +36,7 @@ test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository
 	test_must_fail git commit -m second 2>out.commit
 '
 
-test_lazy_prereq COMMIT_OUT 'test -e "$TRASH_DIRECTORY"/out.commit'
+test_lazy_prereq COMMIT_OUT 'test_path_exists "$TRASH_DIRECTORY/out.commit"'
 test_expect_success COMMIT_OUT 'commit output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
@@ -52,7 +52,7 @@ test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repo
 	test_must_fail git update-index file 2>out.update-index
 '
 
-test_lazy_prereq UPDATE_INDEX_OUT 'test -e "$TRASH_DIRECTORY"/out.update-index'
+test_lazy_prereq UPDATE_INDEX_OUT 'test_path_exists "$TRASH_DIRECTORY/out.update-index"'
 test_expect_success UPDATE_INDEX_OUT 'update-index output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
@@ -69,7 +69,7 @@ test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '
 	test_must_fail git add file 2>out.add
 '
 
-test_lazy_prereq ADD_OUT 'test -e "$TRASH_DIRECTORY"/out.add'
+test_lazy_prereq ADD_OUT 'test_path_exists "$TRASH_DIRECTORY/out.add"'
 test_expect_success ADD_OUT 'add output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
-- 
2.50.1 (Apple Git-155)

[PATCH] t0004: replace test -e with test_path_exists

[PRASHANT S BISHT]

Replace old-style path existence checks with the modern test_path_exists
helper function that provides clearer diagnostic messages on failure.
When test -e fails, the output gives no indication of what went wrong.

These instances were found using:

  git grep "test -[efd]" t/ | grep -v "if test"

as suggested in the microproject ideas.

Signed-off-by: PRASHANT S BISHT <prashantjee2025@gmail.com>
---
 t/t0004-unwritable.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
index 3bdafbae0f..2a9fc781b6 100755
--- a/t/t0004-unwritable.sh
+++ b/t/t0004-unwritable.sh
@@ -21,7 +21,7 @@ test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable reposi
 	test_must_fail git write-tree 2>out.write-tree
 '
 
-test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
+test_lazy_prereq WRITE_TREE_OUT 'test_path_exists "$TRASH_DIRECTORY/out.write-tree"'
 test_expect_success WRITE_TREE_OUT 'write-tree output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
@@ -36,7 +36,7 @@ test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository
 	test_must_fail git commit -m second 2>out.commit
 '
 
-test_lazy_prereq COMMIT_OUT 'test -e "$TRASH_DIRECTORY"/out.commit'
+test_lazy_prereq COMMIT_OUT 'test_path_exists "$TRASH_DIRECTORY/out.commit"'
 test_expect_success COMMIT_OUT 'commit output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
@@ -52,7 +52,7 @@ test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repo
 	test_must_fail git update-index file 2>out.update-index
 '
 
-test_lazy_prereq UPDATE_INDEX_OUT 'test -e "$TRASH_DIRECTORY"/out.update-index'
+test_lazy_prereq UPDATE_INDEX_OUT 'test_path_exists "$TRASH_DIRECTORY/out.update-index"'
 test_expect_success UPDATE_INDEX_OUT 'update-index output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
@@ -69,7 +69,7 @@ test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '
 	test_must_fail git add file 2>out.add
 '
 
-test_lazy_prereq ADD_OUT 'test -e "$TRASH_DIRECTORY"/out.add'
+test_lazy_prereq ADD_OUT 'test_path_exists "$TRASH_DIRECTORY/out.add"'
 test_expect_success ADD_OUT 'add output on unwritable repository' '
 	cat >expect <<-\EOF &&
 	error: insufficient permission for adding an object to repository database .git/objects
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] t0004: replace test -e with test_path_exists

[Eric Sunshine]

On Mon, Mar 9, 2026 at 2:02 PM PRASHANT S BISHT
<prashantjee2025@gmail.com> wrote:
> Replace old-style path existence checks with the modern test_path_exists
> helper function that provides clearer diagnostic messages on failure.
> When test -e fails, the output gives no indication of what went wrong.
>
> These instances were found using:
>
>   git grep "test -[efd]" t/ | grep -v "if test"
>
> as suggested in the microproject ideas.
>
> Signed-off-by: PRASHANT S BISHT <prashantjee2025@gmail.com>
> ---
> diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
> @@ -21,7 +21,7 @@ test_expect_success POSIXPERM,SANITY 'write-tree should notice unwritable reposi
> -test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
> +test_lazy_prereq WRITE_TREE_OUT 'test_path_exists "$TRASH_DIRECTORY/out.write-tree"'
> @@ -36,7 +36,7 @@ test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository
> -test_lazy_prereq COMMIT_OUT 'test -e "$TRASH_DIRECTORY"/out.commit'
> +test_lazy_prereq COMMIT_OUT 'test_path_exists "$TRASH_DIRECTORY/out.commit"'
> @@ -52,7 +52,7 @@ test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repo
> -test_lazy_prereq UPDATE_INDEX_OUT 'test -e "$TRASH_DIRECTORY"/out.update-index'
> +test_lazy_prereq UPDATE_INDEX_OUT 'test_path_exists "$TRASH_DIRECTORY/out.update-index"'
> @@ -69,7 +69,7 @@ test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '
> -test_lazy_prereq ADD_OUT 'test -e "$TRASH_DIRECTORY"/out.add'
> +test_lazy_prereq ADD_OUT 'test_path_exists "$TRASH_DIRECTORY/out.add"'

I'm afraid you chose instances of `test -e` which should not be
converted to `test_path_exists`.

If you're not familiar with Git's test "prereq" facility, then it is
not obvious, but each of these uses of `test -e` is, in fact, used for
control-flow rather than being used to assert some truth. Hence, these
changes run afoul of point #3 under the "Modernize Test Path Checking
in Git’s Test Suite" item on the SoC microproject ideas page[*].

Please try converting some other cases in some other test script.

[*]: https://git.github.io/SoC-2026-Microprojects/

Re: [PATCH] t0004: replace test -e with test_path_exists

[Junio C Hamano]

PRASHANT S BISHT <prashantjee2025@gmail.com> writes:

> -test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
> +test_lazy_prereq WRITE_TREE_OUT 'test_path_exists "$TRASH_DIRECTORY/out.write-tree"'

I suspect this is utterly wrong.  As you wrote in the proposed log
message, test_path_exists is *NOT* about checking if the path
exists.  It rather is about *expecting* for the path to exist, and
fail *LOUDLY* if it does not.

You need to _think_ if we want a LOUD failure when somebody checks
if a path exists and conditionally skip setting a test prerequisite
when the path does not exist.  The original code is trying to be
quiet, as the check is done not because existence of the checked
path is good and lack of it is a test failure.  Lack of the path is
expected on places where the prerequisite is not set, and that by
itself is not a test failure that you want a LOUD report about.

Re: [PATCH] t0004: replace test -e with test_path_exists

[Jeff King]

On Mon, Mar 09, 2026 at 02:14:10PM -0700, Junio C Hamano wrote:

> PRASHANT S BISHT <prashantjee2025@gmail.com> writes:
> 
> > -test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
> > +test_lazy_prereq WRITE_TREE_OUT 'test_path_exists "$TRASH_DIRECTORY/out.write-tree"'
> 
> I suspect this is utterly wrong.  As you wrote in the proposed log
> message, test_path_exists is *NOT* about checking if the path
> exists.  It rather is about *expecting* for the path to exist, and
> fail *LOUDLY* if it does not.
> 
> You need to _think_ if we want a LOUD failure when somebody checks
> if a path exists and conditionally skip setting a test prerequisite
> when the path does not exist.  The original code is trying to be
> quiet, as the check is done not because existence of the checked
> path is good and lack of it is a test failure.  Lack of the path is
> expected on places where the prerequisite is not set, and that by
> itself is not a test failure that you want a LOUD report about.

I'm not sure I agree. Verbose prereq blocks can help with debugging.
Normally you would not see them at all, but if you are investigating why
a prereq did not trigger, you may want more output.

Without "-v" you would not see the output either way, like:

  ok 1 # skip some test (missing FOO)

But with it, it is the difference between:

  checking prerequisite: FOO
  
  mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
  (
  	cd "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
  	test -e foo
  
  )
  prerequisite FOO not satisfied
  ok 1 # skip some test (missing FOO)

and:

  checking prerequisite: FOO
  
  mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
  (
  	cd "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
  	test_path_exists foo
  
  )
  Path foo doesn't exist
  prerequisite FOO not satisfied
  ok 1 # skip some test (missing FOO)

Probably it's pretty obvious for a one-liner like this, but I think it
would help for a longer block.

-Peff

Re: [PATCH] t0004: replace test -e with test_path_exists

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Without "-v" you would not see the output either way, like:
>
>   ok 1 # skip some test (missing FOO)
>
> But with it, it is the difference between:
>
>   checking prerequisite: FOO
>   
>   mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
>   (
>   	cd "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
>   	test -e foo
>   
>   )
>   prerequisite FOO not satisfied
>   ok 1 # skip some test (missing FOO)
>
> and:
>
>   checking prerequisite: FOO
>   
>   mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
>   (
>   	cd "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
>   	test_path_exists foo
>   
>   )
>   Path foo doesn't exist
>   prerequisite FOO not satisfied
>   ok 1 # skip some test (missing FOO)

Sorry, but I am not convinced.

It is as if satisfying FOO is the norm, and not satisifying FOO,
i.e., missing path "foo", is something worth reporting about.

If the test reported both success and failure loudly, it may be a
different story, though.

> Probably it's pretty obvious for a one-liner like this, but I think it
> would help for a longer block.
>
> -Peff