Re: [PATCH v2][GSoC] t9160:modernize test path checking

>
> Hoda Salim <hoda.s.salim@gmail.com> writes:
>
> > From: HodaSalim <hoda.s.salim@gmail.com>
> >
> > 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).
>
>
> >
> > Signed-off-by: HodaSalim <hoda.s.salim@gmail.com>
> > ---
> >  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"
> >  '