Re: [PATCH] t1410: modernize test path checks

From: Christian Couder <christian.couder@gmail.com>
Date: Oct 9, 2025, 17:04 UTC
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:

Show 8 quoted lines

>
> 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.

Show 7 quoted lines

> ---
>     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!

Show 18 quoted lines

>     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.

Show 19 quoted lines

>  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.