Re: [GSoC PATCH] t3310: replace test -f/-d with test_path_is_file/test_path_is_dir

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 27, 2026, 19:31 UTC
Message-ID: <xmqqv7fioueg.fsf@gitster.g>
In-Reply-To: <20260227184538.4718-1-francescopaparatto@gmail.com>

Francesco Paparatto <francescopaparatto@gmail.com> writes:

Show 5 quoted lines

> Replace old-style path assertions with modern helpers that provide
> clearer diagnostic messages on failure. That's because when test -f fails, the
> output gives no indication of what went wrong.
>
> These instances were found using: git grep "test -[efd]" t/ as suggested in the microproject ideas.

An overly long line. Keep the display columns for a line in a patch e-mail below ~70 columns, so that after getting quoted "> " for a few times, the result will still fit in 80-column terminals.

>  test_expect_success 'cannot do merge w/conflicts when previous merge is unfinished' '
> -	test -d .git/NOTES_MERGE_WORKTREE &&
> +	test_path_is_dir .git/NOTES_MERGE_WORKTREE &&

OK.

Show 6 quoted lines

> @@ -320,7 +320,7 @@ w notes on 1st commit
>  EOF
>  
>  test_expect_success 'can do merge without conflicts even if previous merge is unfinished (x => w)' '
> -	test -d .git/NOTES_MERGE_WORKTREE &&
> +	test_path_is_dir .git/NOTES_MERGE_WORKTREE &&

OK.

Show 12 quoted lines

> @@ -564,10 +564,10 @@ EOF
>  	# NOTES_MERGE_* refs and .git/NOTES_MERGE_* state files must remain
>  	git rev-parse --verify NOTES_MERGE_PARTIAL &&
>  	git rev-parse --verify NOTES_MERGE_REF &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> -	test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
> +	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
> +	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
> +	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> +	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&

OK.

>  	# Refs are unchanged
>  	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
>  	test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&

Here is a pair of opportunities for others that want more microproject exercises to make sure we do not ignore failing "git" invocations.

You do not want to address them, of course. You've had your share already ;-)

Thanks.