From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mon, 09 Mar 2026 19:30:03 GMT
Subject: Re: [PATCH] t0004: replace test -e with test_path_exists
Message-ID: <CAPig+cSp1634HVrj6qVfRodR=J3UubYa_aa32JJSTCe1u2NwDA@mail.gmail.com>
In-Reply-To: <20260309175621.32211-1-prashantjee2025@gmail.com>

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/