From: Christian Couder <christian.couder@gmail.com>
Date: Thu, 09 Oct 2025 17:04:03 GMT
Subject: Re: [PATCH] t1410: modernize test path checks
Message-ID: <CAP8UFD0wa6s1z==dVhuiCALeC7X56CS50qMhoPAdTzT-qy6wXA@mail.gmail.com>
In-Reply-To: <pull.2066.git.git.1759741252581.gitgitgadget@gmail.com>

On Mon, Oct 6, 2025 at 11:01 AM Imvedansh via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Imvedansh <veds17007@gmail.com>
>
> Convert old-style "test -f" and "! test -f" checks to use the
> modern helper functions 'test_path_is_file' and
> 'test_path_is_missing' in t/t1410-reflog.sh.
>
> This improves readability and consistency in the test suite.

I think that it might also improve the error message when a test fails, right?

> Signed-off-by: Imvedansh <veds17007@gmail.com>

We prefer when the "Signed-off-by:" contains the full real name of the
person contributing.

"Documentation/SubmittingPatches" says:

[[real-name]]
Please use a known identity in the `Signed-off-by` trailer, since we cannot
accept anonymous contributions. It is common, but not required, to use some form
of your real name. We realize that some contributors are not comfortable doing
so or prefer to contribute under a pseudonym or preferred name and we can accept
your patch either way, as long as the name and email you use are distinctive,
identifying, and not misleading.

The goal of this policy is to allow us to have sufficient information to contact
you if questions arise about your contribution.


> ---
>     t1410: modernize test path checks
>
>     Hello,
>
>     I'm Vedansh and I'm interested in contributing to Git through Outreachy
>     2025.

Thanks for your interest in contributing to Git!

>     I have successfully built Git from source on Ubuntu (via WSL2) and run
>     the test suite. All tests pass.
>
>     For my microproject, I'd like to modernize the path checking in
>     t/t1410-reflog.sh by replacing 'test -f' with test_path_is_file in lines
>     133-136 (in the 'rewind' test).
>
>     I found 4 instances that are assertions (part of && chains):
>
>      * test -f C
>      * test -f A/B/E
>      * ! test -f F
>      * ! test -f A/G
>
>     I've verified these are test assertions, not flow control statements,
>     and the test currently passes on my system.
>
>     Is this appropriate for a microproject?

It seems to me that Usman already answered this.

>  t/t1410-reflog.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index e30f87a358..ce71f9a30a 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -130,10 +130,10 @@ test_expect_success 'pass through -- to sub-command' '
>
>  test_expect_success rewind '
>         test_tick && git reset --hard HEAD~2 &&
> -       test -f C &&
> -       test -f A/B/E &&
> -       ! test -f F &&
> -       ! test -f A/G &&
> +       test_path_is_file C &&
> +       test_path_is_file A/B/E &&
> +       test_path_is_missing F &&
> +       test_path_is_missing A/G &&

This looks good to me.