threads / patch / 65090

patcht3310: replace test -f/-d with test_path_is_file/test_path_is_dir

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

## tl;dr

4 messages between Feb 27, 2026 and Feb 28, 2026. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Francesco Paparatto· Feb 27, 2026, 18:45 UTC · lore

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.

Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Show changes to t/t3310-notes-merge-manual-resolve.sh +6 −6

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 597df5ebc0..f0054b0a39 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -260,7 +260,7 @@ test_expect_success 'change notes in z' '
 '
 
 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 &&
 	test_must_fail git notes merge z >output 2>&1 &&
 	# Output should indicate what is wrong
 	test_grep -q "\\.git/NOTES_MERGE_\\* exists" output
@@ -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 &&
 	git notes merge x &&
 	verify_notes w &&
 	# Verify that other notes refs has not changed (x and y)
@@ -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 &&
 	# 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)" &&

-- 
2.52.0

Junio C Hamano· Feb 27, 2026, 19:31 UTC · re: Francesco Paparatto · lore

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

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.

Francesco Paparatto· Feb 28, 2026, 00:52 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:

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

Thanks for the review. I'll resend with the commit message line wrapped.

Francesco

Francesco Paparatto· Feb 28, 2026, 00:59 UTC · re: Junio C Hamano · lore

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

Replace old-style path assertions with modern helpers that provide clearer diagnostic messages on failure. 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.

Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Show changes to t/t3310-notes-merge-manual-resolve.sh +6 −6

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 597df5ebc0..f0054b0a39 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -260,7 +260,7 @@ test_expect_success 'change notes in z' '
 '
 
 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 &&
 	test_must_fail git notes merge z >output 2>&1 &&
 	# Output should indicate what is wrong
 	test_grep -q "\\.git/NOTES_MERGE_\\* exists" output
@@ -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 &&
 	git notes merge x &&
 	verify_notes w &&
 	# Verify that other notes refs has not changed (x and y)
@@ -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 &&
 	# 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)" &&

-- 
2.52.0