threads / patch / 63650

v3t2400: replace 'test -[efd]' with 'test_path_is_*'

Subject: [PATCH v3] t2400: replace 'test -[efd]' with 'test_path_is_*'

## tl;dr

2 messages between Jun 16, 2025 and Jun 16, 2025. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Rodrigo Michelassi· Jun 16, 2025, 14:45 UTC · lore

From: rodrigocmichelassi <rodmichelassi@gmail.com>

Sorry for the wrong commit message on V2. 'test_path_is_file', 'test_path_is_dir' and 'test_file_is_missing' are modern path checking methods in Git's development, that emit useful diagnostic information when detect a failing condition, while test -[efd] does not. Replace the basic shell commands 'test -f', 'test -d' and 'test -e', with this modern path checking approach.

Co-authored-by: Isabella Caselli <icaselli@usp.br>
Signed-off-by: Isabella Caselli <icaselli@usp.br>
Signed-off-by: Rodrigo Michelassi <rodmichelassi@gmail.com>
---
 t/t2400-worktree-add.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Show changes to t/t2400-worktree-add.sh +10 −10

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 90638fa886..023e1301c8 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -42,8 +42,8 @@ test_expect_success '"add" using - shorthand' '
 
 test_expect_success '"add" refuses to checkout locked branch' '
 	test_must_fail git worktree add zere main &&
-	! test -d zere &&
-	! test -d .git/worktrees/zere
+	test_path_is_missing zere &&
+	test_path_is_missing .git/worktrees/zere
 '
 
 test_expect_success 'checking out paths not complaining about linked checkouts' '
@@ -70,14 +70,14 @@ test_expect_success '"add" worktree' '
 test_expect_success '"add" worktree with lock' '
 	git worktree add --detach --lock here-with-lock main &&
 	test_when_finished "git worktree unlock here-with-lock || :" &&
-	test -f .git/worktrees/here-with-lock/locked
+	test_path_is_file .git/worktrees/here-with-lock/locked
 '
 
 test_expect_success '"add" worktree with lock and reason' '
 	lock_reason="why not" &&
 	git worktree add --detach --lock --reason "$lock_reason" here-with-lock-reason main &&
 	test_when_finished "git worktree unlock here-with-lock-reason || :" &&
-	test -f .git/worktrees/here-with-lock-reason/locked &&
+	test_path_is_file .git/worktrees/here-with-lock-reason/locked &&
 	echo "$lock_reason" >expect &&
 	test_cmp expect .git/worktrees/here-with-lock-reason/locked
 '
@@ -412,14 +412,14 @@ test_expect_success '"add --orphan" with empty repository' '
 test_expect_success '"add" worktree with orphan branch and lock' '
 	git worktree add --lock --orphan -b orphanbr orphan-with-lock &&
 	test_when_finished "git worktree unlock orphan-with-lock || :" &&
-	test -f .git/worktrees/orphan-with-lock/locked
+	test_path_is_file .git/worktrees/orphan-with-lock/locked
 '
 
 test_expect_success '"add" worktree with orphan branch, lock, and reason' '
 	lock_reason="why not" &&
 	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
 	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
-	test -f .git/worktrees/orphan-with-lock-reason/locked &&
+	test_path_is_file .git/worktrees/orphan-with-lock-reason/locked &&
 	echo "$lock_reason" >expect &&
 	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
 '
@@ -474,7 +474,7 @@ test_expect_success 'local clone --shared from linked checkout' '
 
 test_expect_success '"add" worktree with --no-checkout' '
 	git worktree add --no-checkout -b swamp swamp &&
-	! test -e swamp/init.t &&
+	test_path_is_missing swamp/init.t &&
 	git -C swamp reset --hard &&
 	test_cmp init.t swamp/init.t
 '
@@ -497,7 +497,7 @@ test_expect_success 'put a worktree under rebase' '
 
 test_expect_success 'add a worktree, checking out a rebased branch' '
 	test_must_fail git worktree add new-rebase under-rebase &&
-	! test -d new-rebase
+	test_path_is_missing new-rebase
 '
 
 test_expect_success 'checking out a rebased branch from another worktree' '
@@ -535,7 +535,7 @@ test_expect_success 'checkout a branch under bisect' '
 		git worktree list >actual &&
 		grep "under-bisect.*detached HEAD" actual &&
 		test_must_fail git worktree add new-bisect under-bisect &&
-		! test -d new-bisect
+		test_path_is_missing new-bisect
 	)
 '
 
@@ -1165,7 +1165,7 @@ test_expect_success '"add" not tripped up by magic worktree matching"' '
 
 test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
 	git worktree add --detach ".  weird*..?.lock.lock" &&
-	test -d .git/worktrees/---weird-.-
+	test_path_is_dir .git/worktrees/---weird-.-
 '
 
 test_expect_success '"add" should not fail because of another bad worktree' '

-- 
2.50.0.rc2.48.gf1ca98f609.dirty

Junio C Hamano· Jun 16, 2025, 18:00 UTC · re: Rodrigo Michelassi · lore

Re: [PATCH v3] t2400: replace 'test -[efd]' with 'test_path_is_*'

Rodrigo Michelassi <rodmichelassi@gmail.com> writes:

> From: rodrigocmichelassi <rodmichelassi@gmail.com>

As pointed out earlier, we want to see this line look like

    From: Rodrigo Michelassi <rodmichelassi@gmail.com>

This often comes from your commit object; you'd have to fix it there to match "Rodrigo Michelassi <rodmichelassi@gmail.com>" used to sign off this patch, something like:

 $ git commit --no-edit --amend \
   --author="Rodrigo Michelassi <rodmichelassi@gmail.com>"

while you have that commit checked out, and then

 $ git format-patch >patch.txt --stdout -v4 -1 HEAD

to prepare the patch, proofread it and then optionally after fixing anything you found problematic in your proofreading, run git-send-email on it.

Because your MUA already gives the name you use on your signed-off-by line on the From: line, optionally you can just edit the line out before sending, i.e. after the format-patch produces the text file, before send-email sends the file out.

> Sorry for the wrong commit message on V2.

Anything you write in the body of the message, before the three-dash line, goes to the resulting commit log message. You do not want to etch the above message into stone. You can write such an ephemeral message after the three-dash lines. See these for examples.

    https://lore.kernel.org/git/20250512185233.GC1276214@coredump.intra.peff.net/
    https://lore.kernel.org/git/20250225063421.GJ1293961@coredump.intra.peff.net/

> 'test_path_is_file', 'test_path_is_dir' and 'test_file_is_missing'
> are modern path checking methods in Git's development, that emit
> useful diagnostic information when detect a failing condition, while

"modern path checking methods" -> "test helpers used"

Also, "when detect" -> "when they detect", probably.

> test -[efd] does not.
> Replace the basic shell commands 'test -f', 'test -d' and 'test -e',
> with this modern path checking approach.

Ditto.

Show 6 quoted lines

> Co-authored-by: Isabella Caselli <icaselli@usp.br>
> Signed-off-by: Isabella Caselli <icaselli@usp.br>
> Signed-off-by: Rodrigo Michelassi <rodmichelassi@gmail.com>
> ---
>  t/t2400-worktree-add.sh | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

The changes to the test look good to me.

Thanks.

Show 85 quoted lines

> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index 90638fa886..023e1301c8 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -42,8 +42,8 @@ test_expect_success '"add" using - shorthand' '
>  
>  test_expect_success '"add" refuses to checkout locked branch' '
>  	test_must_fail git worktree add zere main &&
> -	! test -d zere &&
> -	! test -d .git/worktrees/zere
> +	test_path_is_missing zere &&
> +	test_path_is_missing .git/worktrees/zere
>  '
>  
>  test_expect_success 'checking out paths not complaining about linked checkouts' '
> @@ -70,14 +70,14 @@ test_expect_success '"add" worktree' '
>  test_expect_success '"add" worktree with lock' '
>  	git worktree add --detach --lock here-with-lock main &&
>  	test_when_finished "git worktree unlock here-with-lock || :" &&
> -	test -f .git/worktrees/here-with-lock/locked
> +	test_path_is_file .git/worktrees/here-with-lock/locked
>  '
>  
>  test_expect_success '"add" worktree with lock and reason' '
>  	lock_reason="why not" &&
>  	git worktree add --detach --lock --reason "$lock_reason" here-with-lock-reason main &&
>  	test_when_finished "git worktree unlock here-with-lock-reason || :" &&
> -	test -f .git/worktrees/here-with-lock-reason/locked &&
> +	test_path_is_file .git/worktrees/here-with-lock-reason/locked &&
>  	echo "$lock_reason" >expect &&
>  	test_cmp expect .git/worktrees/here-with-lock-reason/locked
>  '
> @@ -412,14 +412,14 @@ test_expect_success '"add --orphan" with empty repository' '
>  test_expect_success '"add" worktree with orphan branch and lock' '
>  	git worktree add --lock --orphan -b orphanbr orphan-with-lock &&
>  	test_when_finished "git worktree unlock orphan-with-lock || :" &&
> -	test -f .git/worktrees/orphan-with-lock/locked
> +	test_path_is_file .git/worktrees/orphan-with-lock/locked
>  '
>  
>  test_expect_success '"add" worktree with orphan branch, lock, and reason' '
>  	lock_reason="why not" &&
>  	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
>  	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
> -	test -f .git/worktrees/orphan-with-lock-reason/locked &&
> +	test_path_is_file .git/worktrees/orphan-with-lock-reason/locked &&
>  	echo "$lock_reason" >expect &&
>  	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
>  '
> @@ -474,7 +474,7 @@ test_expect_success 'local clone --shared from linked checkout' '
>  
>  test_expect_success '"add" worktree with --no-checkout' '
>  	git worktree add --no-checkout -b swamp swamp &&
> -	! test -e swamp/init.t &&
> +	test_path_is_missing swamp/init.t &&
>  	git -C swamp reset --hard &&
>  	test_cmp init.t swamp/init.t
>  '
> @@ -497,7 +497,7 @@ test_expect_success 'put a worktree under rebase' '
>  
>  test_expect_success 'add a worktree, checking out a rebased branch' '
>  	test_must_fail git worktree add new-rebase under-rebase &&
> -	! test -d new-rebase
> +	test_path_is_missing new-rebase
>  '
>  
>  test_expect_success 'checking out a rebased branch from another worktree' '
> @@ -535,7 +535,7 @@ test_expect_success 'checkout a branch under bisect' '
>  		git worktree list >actual &&
>  		grep "under-bisect.*detached HEAD" actual &&
>  		test_must_fail git worktree add new-bisect under-bisect &&
> -		! test -d new-bisect
> +		test_path_is_missing new-bisect
>  	)
>  '
>  
> @@ -1165,7 +1165,7 @@ test_expect_success '"add" not tripped up by magic worktree matching"' '
>  
>  test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
>  	git worktree add --detach ".  weird*..?.lock.lock" &&
> -	test -d .git/worktrees/---weird-.-
> +	test_path_is_dir .git/worktrees/---weird-.-
>  '
>  
>  test_expect_success '"add" should not fail because of another bad worktree' '