{"thread":{"id":"63650","subject":"[PATCH v3] t2400: replace 'test -[efd]' with 'test_path_is_*'","startedAt":"2025-06-16T14:45:45Z","lastAt":"2025-06-16T18:00:55Z","messageCount":2,"participants":["Rodrigo Michelassi","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"520289","messageId":"20250616144540.21075-1-rodmichelassi@gmail.com","threadId":"63650","inReplyTo":null,"subject":"[PATCH v3] t2400: replace 'test -[efd]' with 'test_path_is_*'","fromName":"Rodrigo Michelassi","fromEmail":"rodmichelassi@gmail.com","sentAt":"2025-06-16T14:45:40Z","receivedAt":"2025-06-16T14:45:45Z","isPatch":true,"sender":{"key":"rodmichelassi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/101156335?v=4"},"body":"From: rodrigocmichelassi <rodmichelassi@gmail.com>\n\nSorry for the wrong commit message on V2.\n'test_path_is_file', 'test_path_is_dir' and 'test_file_is_missing'\nare modern path checking methods in Git's development, that emit\nuseful diagnostic information when detect a failing condition, while\ntest -[efd] does not.\nReplace the basic shell commands 'test -f', 'test -d' and 'test -e',\nwith this modern path checking approach.\n\nCo-authored-by: Isabella Caselli <icaselli@usp.br>\nSigned-off-by: Isabella Caselli <icaselli@usp.br>\nSigned-off-by: Rodrigo Michelassi <rodmichelassi@gmail.com>\n---\n t/t2400-worktree-add.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 90638fa886..023e1301c8 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -42,8 +42,8 @@ test_expect_success '\"add\" using - shorthand' '\n \n test_expect_success '\"add\" refuses to checkout locked branch' '\n \ttest_must_fail git worktree add zere main &&\n-\t! test -d zere &&\n-\t! test -d .git/worktrees/zere\n+\ttest_path_is_missing zere &&\n+\ttest_path_is_missing .git/worktrees/zere\n '\n \n test_expect_success 'checking out paths not complaining about linked checkouts' '\n@@ -70,14 +70,14 @@ test_expect_success '\"add\" worktree' '\n test_expect_success '\"add\" worktree with lock' '\n \tgit worktree add --detach --lock here-with-lock main &&\n \ttest_when_finished \"git worktree unlock here-with-lock || :\" &&\n-\ttest -f .git/worktrees/here-with-lock/locked\n+\ttest_path_is_file .git/worktrees/here-with-lock/locked\n '\n \n test_expect_success '\"add\" worktree with lock and reason' '\n \tlock_reason=\"why not\" &&\n \tgit worktree add --detach --lock --reason \"$lock_reason\" here-with-lock-reason main &&\n \ttest_when_finished \"git worktree unlock here-with-lock-reason || :\" &&\n-\ttest -f .git/worktrees/here-with-lock-reason/locked &&\n+\ttest_path_is_file .git/worktrees/here-with-lock-reason/locked &&\n \techo \"$lock_reason\" >expect &&\n \ttest_cmp expect .git/worktrees/here-with-lock-reason/locked\n '\n@@ -412,14 +412,14 @@ test_expect_success '\"add --orphan\" with empty repository' '\n test_expect_success '\"add\" worktree with orphan branch and lock' '\n \tgit worktree add --lock --orphan -b orphanbr orphan-with-lock &&\n \ttest_when_finished \"git worktree unlock orphan-with-lock || :\" &&\n-\ttest -f .git/worktrees/orphan-with-lock/locked\n+\ttest_path_is_file .git/worktrees/orphan-with-lock/locked\n '\n \n test_expect_success '\"add\" worktree with orphan branch, lock, and reason' '\n \tlock_reason=\"why not\" &&\n \tgit worktree add --detach --lock --reason \"$lock_reason\" orphan-with-lock-reason main &&\n \ttest_when_finished \"git worktree unlock orphan-with-lock-reason || :\" &&\n-\ttest -f .git/worktrees/orphan-with-lock-reason/locked &&\n+\ttest_path_is_file .git/worktrees/orphan-with-lock-reason/locked &&\n \techo \"$lock_reason\" >expect &&\n \ttest_cmp expect .git/worktrees/orphan-with-lock-reason/locked\n '\n@@ -474,7 +474,7 @@ test_expect_success 'local clone --shared from linked checkout' '\n \n test_expect_success '\"add\" worktree with --no-checkout' '\n \tgit worktree add --no-checkout -b swamp swamp &&\n-\t! test -e swamp/init.t &&\n+\ttest_path_is_missing swamp/init.t &&\n \tgit -C swamp reset --hard &&\n \ttest_cmp init.t swamp/init.t\n '\n@@ -497,7 +497,7 @@ test_expect_success 'put a worktree under rebase' '\n \n test_expect_success 'add a worktree, checking out a rebased branch' '\n \ttest_must_fail git worktree add new-rebase under-rebase &&\n-\t! test -d new-rebase\n+\ttest_path_is_missing new-rebase\n '\n \n test_expect_success 'checking out a rebased branch from another worktree' '\n@@ -535,7 +535,7 @@ test_expect_success 'checkout a branch under bisect' '\n \t\tgit worktree list >actual &&\n \t\tgrep \"under-bisect.*detached HEAD\" actual &&\n \t\ttest_must_fail git worktree add new-bisect under-bisect &&\n-\t\t! test -d new-bisect\n+\t\ttest_path_is_missing new-bisect\n \t)\n '\n \n@@ -1165,7 +1165,7 @@ test_expect_success '\"add\" not tripped up by magic worktree matching\"' '\n \n test_expect_success FUNNYNAMES 'sanitize generated worktree name' '\n \tgit worktree add --detach \".  weird*..?.lock.lock\" &&\n-\ttest -d .git/worktrees/---weird-.-\n+\ttest_path_is_dir .git/worktrees/---weird-.-\n '\n \n test_expect_success '\"add\" should not fail because of another bad worktree' '\n-- \n2.50.0.rc2.48.gf1ca98f609.dirty\n\n"},{"id":"520293","messageId":"xmqqo6unzhuz.fsf@gitster.g","threadId":"63650","inReplyTo":"20250616144540.21075-1-rodmichelassi@gmail.com","subject":"Re: [PATCH v3] t2400: replace 'test -[efd]' with 'test_path_is_*'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-16T18:00:52Z","receivedAt":"2025-06-16T18:00:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rodrigo Michelassi <rodmichelassi@gmail.com> writes:\n\n> From: rodrigocmichelassi <rodmichelassi@gmail.com>\n\nAs pointed out earlier, we want to see this line look like\n\n    From: Rodrigo Michelassi <rodmichelassi@gmail.com>\n\nThis often comes from your commit object; you'd have to fix it there\nto match \"Rodrigo Michelassi <rodmichelassi@gmail.com>\" used to sign\noff this patch, something like:\n\n $ git commit --no-edit --amend \\\n   --author=\"Rodrigo Michelassi <rodmichelassi@gmail.com>\"\n\nwhile you have that commit checked out, and then\n\n $ git format-patch >patch.txt --stdout -v4 -1 HEAD\n\nto prepare the patch, proofread it and then optionally after fixing\nanything you found problematic in your proofreading, run git-send-email\non it.\n\nBecause your MUA already gives the name you use on your\nsigned-off-by line on the From: line, optionally you can just edit\nthe line out before sending, i.e. after the format-patch produces\nthe text file, before send-email sends the file out.\n\n> Sorry for the wrong commit message on V2.\n\nAnything you write in the body of the message, before the three-dash\nline, goes to the resulting commit log message.  You do not want to\netch the above message into stone.  You can write such an ephemeral\nmessage after the three-dash lines.  See these for examples.\n\n    https://lore.kernel.org/git/20250512185233.GC1276214@coredump.intra.peff.net/\n    https://lore.kernel.org/git/20250225063421.GJ1293961@coredump.intra.peff.net/\n\n> 'test_path_is_file', 'test_path_is_dir' and 'test_file_is_missing'\n> are modern path checking methods in Git's development, that emit\n> useful diagnostic information when detect a failing condition, while\n\n\"modern path checking methods\" -> \"test helpers used\"\n\nAlso, \"when detect\" -> \"when they detect\", probably.\n\n> test -[efd] does not.\n> Replace the basic shell commands 'test -f', 'test -d' and 'test -e',\n> with this modern path checking approach.\n\nDitto.\n\n> Co-authored-by: Isabella Caselli <icaselli@usp.br>\n> Signed-off-by: Isabella Caselli <icaselli@usp.br>\n> Signed-off-by: Rodrigo Michelassi <rodmichelassi@gmail.com>\n> ---\n>  t/t2400-worktree-add.sh | 20 ++++++++++----------\n>  1 file changed, 10 insertions(+), 10 deletions(-)\n\nThe changes to the test look good to me.\n\nThanks.\n\n\n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index 90638fa886..023e1301c8 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -42,8 +42,8 @@ test_expect_success '\"add\" using - shorthand' '\n>  \n>  test_expect_success '\"add\" refuses to checkout locked branch' '\n>  \ttest_must_fail git worktree add zere main &&\n> -\t! test -d zere &&\n> -\t! test -d .git/worktrees/zere\n> +\ttest_path_is_missing zere &&\n> +\ttest_path_is_missing .git/worktrees/zere\n>  '\n>  \n>  test_expect_success 'checking out paths not complaining about linked checkouts' '\n> @@ -70,14 +70,14 @@ test_expect_success '\"add\" worktree' '\n>  test_expect_success '\"add\" worktree with lock' '\n>  \tgit worktree add --detach --lock here-with-lock main &&\n>  \ttest_when_finished \"git worktree unlock here-with-lock || :\" &&\n> -\ttest -f .git/worktrees/here-with-lock/locked\n> +\ttest_path_is_file .git/worktrees/here-with-lock/locked\n>  '\n>  \n>  test_expect_success '\"add\" worktree with lock and reason' '\n>  \tlock_reason=\"why not\" &&\n>  \tgit worktree add --detach --lock --reason \"$lock_reason\" here-with-lock-reason main &&\n>  \ttest_when_finished \"git worktree unlock here-with-lock-reason || :\" &&\n> -\ttest -f .git/worktrees/here-with-lock-reason/locked &&\n> +\ttest_path_is_file .git/worktrees/here-with-lock-reason/locked &&\n>  \techo \"$lock_reason\" >expect &&\n>  \ttest_cmp expect .git/worktrees/here-with-lock-reason/locked\n>  '\n> @@ -412,14 +412,14 @@ test_expect_success '\"add --orphan\" with empty repository' '\n>  test_expect_success '\"add\" worktree with orphan branch and lock' '\n>  \tgit worktree add --lock --orphan -b orphanbr orphan-with-lock &&\n>  \ttest_when_finished \"git worktree unlock orphan-with-lock || :\" &&\n> -\ttest -f .git/worktrees/orphan-with-lock/locked\n> +\ttest_path_is_file .git/worktrees/orphan-with-lock/locked\n>  '\n>  \n>  test_expect_success '\"add\" worktree with orphan branch, lock, and reason' '\n>  \tlock_reason=\"why not\" &&\n>  \tgit worktree add --detach --lock --reason \"$lock_reason\" orphan-with-lock-reason main &&\n>  \ttest_when_finished \"git worktree unlock orphan-with-lock-reason || :\" &&\n> -\ttest -f .git/worktrees/orphan-with-lock-reason/locked &&\n> +\ttest_path_is_file .git/worktrees/orphan-with-lock-reason/locked &&\n>  \techo \"$lock_reason\" >expect &&\n>  \ttest_cmp expect .git/worktrees/orphan-with-lock-reason/locked\n>  '\n> @@ -474,7 +474,7 @@ test_expect_success 'local clone --shared from linked checkout' '\n>  \n>  test_expect_success '\"add\" worktree with --no-checkout' '\n>  \tgit worktree add --no-checkout -b swamp swamp &&\n> -\t! test -e swamp/init.t &&\n> +\ttest_path_is_missing swamp/init.t &&\n>  \tgit -C swamp reset --hard &&\n>  \ttest_cmp init.t swamp/init.t\n>  '\n> @@ -497,7 +497,7 @@ test_expect_success 'put a worktree under rebase' '\n>  \n>  test_expect_success 'add a worktree, checking out a rebased branch' '\n>  \ttest_must_fail git worktree add new-rebase under-rebase &&\n> -\t! test -d new-rebase\n> +\ttest_path_is_missing new-rebase\n>  '\n>  \n>  test_expect_success 'checking out a rebased branch from another worktree' '\n> @@ -535,7 +535,7 @@ test_expect_success 'checkout a branch under bisect' '\n>  \t\tgit worktree list >actual &&\n>  \t\tgrep \"under-bisect.*detached HEAD\" actual &&\n>  \t\ttest_must_fail git worktree add new-bisect under-bisect &&\n> -\t\t! test -d new-bisect\n> +\t\ttest_path_is_missing new-bisect\n>  \t)\n>  '\n>  \n> @@ -1165,7 +1165,7 @@ test_expect_success '\"add\" not tripped up by magic worktree matching\"' '\n>  \n>  test_expect_success FUNNYNAMES 'sanitize generated worktree name' '\n>  \tgit worktree add --detach \".  weird*..?.lock.lock\" &&\n> -\ttest -d .git/worktrees/---weird-.-\n> +\ttest_path_is_dir .git/worktrees/---weird-.-\n>  '\n>  \n>  test_expect_success '\"add\" should not fail because of another bad worktree' '\n"}]}