Re: [PATCH v2][GSoC] t9160:modernize test path checking
From: Junio C Hamano <hidden>
Date: 2026-02-02 19:00:24
Hoda Salim [off-list ref] writes:
From: HodaSalim <redacted> Replace old-style path checks with Git's dedicated test helpers: - test -f → test_path_is_file - test -d → test_path_is_dir - test -s → test_file_not_empty Fix typos with the word "subsequent" Found using: git grep "test -[efd]" t/ This improves test readability and provides better error messages when path checks fail.
For a small change like this, the above is sufficient as all the necessary things are described, but for future reference, we prefer to explain things in this order: - Give an observation on how the current system works in the present tense (so no need to say "Currently X is Y", or "Previously X was Y" to describe the state before your change; just "X is Y" is enough), and discuss what you perceive as a problem in it. - Propose a solution (optional---often, problem description trivially leads to an obvious solution in reader's minds). - Give commands to somebody editing the codebase to "make it so", instead of saying "This commit does X". You are going backwards ;-). Will queue. Thanks (and thanks for all the reviewers who helped polish the draft to get to this v2).
quoted hunk
Signed-off-by: HodaSalim <redacted> --- t/t9160-git-svn-preserve-empty-dirs.sh | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)diff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh index 36c6b1a12f..de32cf2542 100755 --- a/t/t9160-git-svn-preserve-empty-dirs.sh +++ b/t/t9160-git-svn-preserve-empty-dirs.sh@@ -61,15 +61,15 @@ test_expect_success 'clone svn repo with --preserve-empty-dirs' ' # "$GIT_REPO"/1 should only contain the placeholder file. test_expect_success 'directory empty from inception' ' - test -f "$GIT_REPO"/1/.gitignore && + test_path_is_file "$GIT_REPO"/1/.gitignore && test $(find "$GIT_REPO"/1 -type f | wc -l) = "1" ' # "$GIT_REPO"/2 and "$GIT_REPO"/3 should only contain the placeholder file. test_expect_success 'directory empty from subsequent svn commit' ' - test -f "$GIT_REPO"/2/.gitignore && + test_path_is_file "$GIT_REPO"/2/.gitignore && test $(find "$GIT_REPO"/2 -type f | wc -l) = "1" && - test -f "$GIT_REPO"/3/.gitignore && + test_path_is_file "$GIT_REPO"/3/.gitignore && test $(find "$GIT_REPO"/3 -type f | wc -l) = "1" '@@ -77,7 +77,7 @@ test_expect_success 'directory empty from subsequent svn commit' ' # generated for every sub-directory at some point in the repo's history. test_expect_success 'add entry to previously empty directory' ' test $(find "$GIT_REPO"/4 -type f | wc -l) = "1" && - test -f "$GIT_REPO"/4/a/b/c/foo + test_path_is_file "$GIT_REPO"/4/a/b/c/foo ' # The HEAD~2 commit should not have introduced .gitignore placeholder files.@@ -102,14 +102,14 @@ test_expect_success 'clone svn repo with --placeholder-file specified' ' # "$GIT_REPO"/5/.placeholder should be a file, and non-empty. test_expect_success 'placeholder namespace conflict with file' ' - test -s "$GIT_REPO"/5/.placeholder + test_file_not_empty "$GIT_REPO"/5/.placeholder ' # "$GIT_REPO"/6/.placeholder should be a directory, and the "$GIT_REPO"/6 tree # should only contain one file: the placeholder. test_expect_success 'placeholder namespace conflict with directory' ' - test -d "$GIT_REPO"/6/.placeholder && - test -f "$GIT_REPO"/6/.placeholder/.placeholder && + test_path_is_dir "$GIT_REPO"/6/.placeholder && + test_path_is_file "$GIT_REPO"/6/.placeholder/.placeholder && test $(find "$GIT_REPO"/6 -type f | wc -l) = "1" '@@ -133,19 +133,19 @@ test_expect_success 'second set of svn commits and rebase' ' # Check that --preserve-empty-dirs and --placeholder-file flag state # stays persistent over multiple invocations. -test_expect_success 'flag persistence during subsqeuent rebase' ' - test -f "$GIT_REPO"/7/.placeholder && +test_expect_success 'flag persistence during subsequent rebase' ' + test_path_is_file "$GIT_REPO"/7/.placeholder && test $(find "$GIT_REPO"/7 -type f | wc -l) = "1" ' # Check that placeholder files are properly removed when unnecessary, # even across multiple invocations. -test_expect_success 'placeholder list persistence during subsqeuent rebase' ' - test -f "$GIT_REPO"/1/file1.txt && +test_expect_success 'placeholder list persistence during subsequent rebase' ' + test_path_is_file "$GIT_REPO"/1/file1.txt && test $(find "$GIT_REPO"/1 -type f | wc -l) = "1" && - test -f "$GIT_REPO"/5/file1.txt && - test -f "$GIT_REPO"/5/.placeholder && + test_path_is_file "$GIT_REPO"/5/file1.txt && + test_path_is_file "$GIT_REPO"/5/.placeholder && test $(find "$GIT_REPO"/5 -type f | wc -l) = "2" '