From: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
Date: Fri, 09 Jan 2026 11:24:14 GMT
Subject: Re: [GSoC] t7101: modernize test path checks
Message-ID: <CALE2CrS0Q2NS1DbFv4pyRQsuypu=KH6Kurs=m4yWrFbR9QosoA@mail.gmail.com>
In-Reply-To: <20260109032027.68680-1-jayatheerthkulkarni2005@gmail.com>

Hi Jayatheerth,

Thanks for working on modernizing this test. Using the test_path_* helpers
is definitely the right direction and makes failures much easier to
diagnose.

I went through the patch and the conversions look correct:

- test -d path0        -> test_path_is_dir path0
- test -f path0/COPYING -> test_path_is_file path0/COPYING
- ! test -f …          -> test_path_is_missing …

all preserve the original intent of the checks while improving diagnostics.

The small cleanups are also helpful:
- Fixing the duplicated “checking” in the test description.
- Correcting the path from path1/COPYING-TOO to path0/COPYING-TOO so it
  matches the actual tree being tested.

I don’t see any semantic changes introduced here; this looks like a clean
and safe modernization of the test.

Thanks for the patch.

Pushkar

On Fri, Jan 9, 2026 at 8:52 AM K Jayatheerth
<jayatheerthkulkarni2005@gmail.com> wrote:
>
> Replace old-style `test -[df]` and `! test -[df]` assertions with
> the modern `test_path_is_file`, `test_path_is_dir`, and
> `test_path_is_missing` helpers.
>
> These helpers provide more informative error messages in case of
> failure (e.g., "File 'foo' is missing" instead of just exit code 1).
>
> While at it, fix a typo and an incorrect path
> reference in one of the test descriptions.
>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  t/t7101-reset-empty-subdirs.sh | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/t/t7101-reset-empty-subdirs.sh b/t/t7101-reset-empty-subdirs.sh
> index 33d5d5b76e..d1d3e231fc 100755
> --- a/t/t7101-reset-empty-subdirs.sh
> +++ b/t/t7101-reset-empty-subdirs.sh
> @@ -34,32 +34,32 @@ test_expect_success 'resetting tree HEAD^' '
>  '
>
>  test_expect_success 'checking initial files exist after rewind' '
> -       test -d path0 &&
> -       test -f path0/COPYING
> +       test_path_is_dir path0 &&
> +       test_path_is_file path0/COPYING
>  '
>
>  test_expect_success 'checking lack of path1/path2/COPYING' '
> -       ! test -f path1/path2/COPYING
> +       test_path_is_missing path1/path2/COPYING
>  '
>
>  test_expect_success 'checking lack of path1/COPYING' '
> -       ! test -f path1/COPYING
> +       test_path_is_missing path1/COPYING
>  '
>
>  test_expect_success 'checking lack of COPYING' '
> -       ! test -f COPYING
> +       test_path_is_missing COPYING
>  '
>
> -test_expect_success 'checking checking lack of path1/COPYING-TOO' '
> -       ! test -f path0/COPYING-TOO
> +test_expect_success 'checking lack of path0/COPYING-TOO' '
> +       test_path_is_missing path0/COPYING-TOO
>  '
>
>  test_expect_success 'checking lack of path1/path2' '
> -       ! test -d path1/path2
> +       test_path_is_missing path1/path2
>  '
>
>  test_expect_success 'checking lack of path1' '
> -       ! test -d path1
> +       test_path_is_missing path1
>  '
>
>  test_done
> --
> 2.52.0
>
>