| Message ID | 20250308090358.25429-1-contact@aynp.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GSoC] t9400: prefer test_path_* helper functions | expand |
On Sat, Mar 08, 2025 at 06:03:58PM +0900, Aryan Pathania wrote: > use `test_path_*` instead of `test -[efd]` to avoid false complaints and Nit: We want to have full sentences in commit messages. Those sentences should start with an upper-case letter and end with punctuation. > output when running the script with `-v` or `-x` Hm. I'm not exactly sure what "false complaints" you are talking about here. The benefit of `test_path_*`()` over `test -[efd]` is that they actually print information _why_ they have failed, which may help devs to figure out what's happening. So it's kind of the opposite of what you say in the commit message: we're now printing _more_ output, not less. > Signed-off-by: Aryan Pathania <contact@aynp.dev> > --- > t/t9400-git-cvsserver-server.sh | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh > index e499c7f955..6c7cb1964c 100755 > --- a/t/t9400-git-cvsserver-server.sh > +++ b/t/t9400-git-cvsserver-server.sh > @@ -254,7 +254,7 @@ test_expect_success 'gitcvs.enabled = false' \ > true > fi && > grep "GITCVS emulation disabled" cvs.log && > - test ! -d cvswork2' > + test_path_is_missing cvswork2' This isn't quite equivalent: we've been checking that the path is not a directory before, but now we verify that the path doesn't exist. It's a sensible change in this context though as our new assertion is stronger than the old one, but it might make sense to point out in the commit message. > @@ -285,7 +285,7 @@ test_expect_success 'gitcvs.dbname' ' > GIT_DIR="$SERVERDIR" git config gitcvs.dbname %Ggitcvs.%a.%m.sqlite && > GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 main >cvs.log 2>&1 && > test_cmp cvswork cvswork2 && > - test -f "$SERVERDIR/gitcvs.ext.main.sqlite" && > + test_path_is_file_not_symlink "$SERVERDIR/gitcvs.ext.main.sqlite" && > cmp "$SERVERDIR/gitcvs.main.sqlite" "$SERVERDIR/gitcvs.ext.main.sqlite" > ' > We tend to use `test_path_is_file` rather than `test_path_is_file_not_symlink`, but I don't mind it too much. Patrick
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index e499c7f955..6c7cb1964c 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -254,7 +254,7 @@ test_expect_success 'gitcvs.enabled = false' \ true fi && grep "GITCVS emulation disabled" cvs.log && - test ! -d cvswork2' + test_path_is_missing cvswork2' rm -fr cvswork2 test_expect_success 'gitcvs.ext.enabled = true' ' @@ -276,7 +276,7 @@ test_expect_success 'gitcvs.ext.enabled = false' ' true fi && grep "GITCVS emulation disabled" cvs.log && - test ! -d cvswork2 + test_path_is_missing cvswork2 ' rm -fr cvswork2 @@ -285,7 +285,7 @@ test_expect_success 'gitcvs.dbname' ' GIT_DIR="$SERVERDIR" git config gitcvs.dbname %Ggitcvs.%a.%m.sqlite && GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 main >cvs.log 2>&1 && test_cmp cvswork cvswork2 && - test -f "$SERVERDIR/gitcvs.ext.main.sqlite" && + test_path_is_file_not_symlink "$SERVERDIR/gitcvs.ext.main.sqlite" && cmp "$SERVERDIR/gitcvs.main.sqlite" "$SERVERDIR/gitcvs.ext.main.sqlite" ' @@ -296,8 +296,8 @@ test_expect_success 'gitcvs.ext.dbname' ' GIT_DIR="$SERVERDIR" git config gitcvs.dbname %Ggitcvs2.%a.%m.sqlite && GIT_CONFIG="$git_config" cvs -Q co -d cvswork2 main >cvs.log 2>&1 && test_cmp cvswork cvswork2 && - test -f "$SERVERDIR/gitcvs1.ext.main.sqlite" && - test ! -f "$SERVERDIR/gitcvs2.ext.main.sqlite" && + test_path_is_file_not_symlink "$SERVERDIR/gitcvs1.ext.main.sqlite" && + test_path_is_missing "$SERVERDIR/gitcvs2.ext.main.sqlite" && cmp "$SERVERDIR/gitcvs.main.sqlite" "$SERVERDIR/gitcvs1.ext.main.sqlite" ' @@ -346,7 +346,7 @@ test_expect_failure "cvs update w/o -d doesn't create subdir (TODO)" ' git push gitcvs.git >/dev/null && cd cvswork && GIT_CONFIG="$git_config" cvs -Q update && - test ! -d test + test_path_is_missing test ' cd "$WORKDIR" @@ -379,7 +379,7 @@ test_expect_success 'cvs update (delete file)' ' cd cvswork && GIT_CONFIG="$git_config" cvs -Q update && test -z "$(grep testfile1 CVS/Entries)" && - test ! -f testfile1 + test_path_is_missing testfile1 ' cd "$WORKDIR"
use `test_path_*` instead of `test -[efd]` to avoid false complaints and output when running the script with `-v` or `-x` Signed-off-by: Aryan Pathania <contact@aynp.dev> --- t/t9400-git-cvsserver-server.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)