# [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*' 9 messages from 2026-07-02 to 2026-07-13. Participants: Marcelo Machado Lage, Patrick Steinhardt, Junio C Hamano. Thread: https://gitlist.dev/t/65914 ## Marcelo Machado Lage, 2026-07-02 14:07 Subject: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*' Message-ID: <20260702140704.65805-1-marcelomlage@usp.br> ``` Replace the basic shell commands 'test -f', with more modern test helpers 'test_path_is_file' and 'test_path_is_missing'. Co-authored-by: Vinicius Lira de Freitas Signed-off-by: Vinicius Lira de Freitas Signed-off-by: Marcelo Machado Lage --- t/t9811-git-p4-label-import.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh index 7614dfbd95..93d6b4c479 100755 --- a/t/t9811-git-p4-label-import.sh +++ b/t/t9811-git-p4-label-import.sh @@ -62,9 +62,9 @@ test_expect_success 'basic p4 labels' ' cd main && git checkout TAG_F1_ONLY && - ! test -f f2 && + test_path_is_missing f2 && git checkout TAG_WITH\$_SHELL_CHAR && - test -f f1 && test -f f2 && test -f file_with_\$metachar && + test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar && git show TAG_LONG_LABEL | grep -q "A Label second line" ) @@ -102,11 +102,11 @@ test_expect_success 'two labels on the same changelist' ' git checkout TAG_F1_1 && ls && - test -f f1 && + test_path_is_file f1 && git checkout TAG_F1_2 && ls && - test -f f1 + test_path_is_file f1 ) ' @@ -135,9 +135,9 @@ test_expect_success 'export git tags to p4' ' p4 labels ... | grep LIGHTWEIGHT_TAG && p4 label -o GIT_TAG_1 | grep "tag created in git:xyzzy" && p4 sync ...@GIT_TAG_1 && - ! test -f main/f10 && + test_path_is_missing main/f10 && p4 sync ...@GIT_TAG_2 && - test -f main/f10 + test_path_is_file main/f10 ) ' @@ -168,9 +168,9 @@ test_expect_success 'export git tags to p4 with deletion' ' cd "$cli" && p4 sync ... && p4 sync ...@GIT_TAG_ON_DELETED && - test -f main/deleted_file && + test_path_is_file main/deleted_file && p4 sync ...@GIT_TAG_AFTER_DELETION && - ! test -f main/deleted_file && + test_path_is_missing main/deleted_file && echo "checking label contents" && p4 label -o GIT_TAG_ON_DELETED | grep "tag on deleted file" ) base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc -- 2.34.1 ``` ## Patrick Steinhardt, 2026-07-03 08:19 Subject: Re: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*' Message-ID: In-Reply-To: <20260702140704.65805-1-marcelomlage@usp.br> ``` On Thu, Jul 02, 2026 at 11:07:04AM -0300, Marcelo Machado Lage wrote: > Replace the basic shell commands 'test -f', with more modern test > helpers 'test_path_is_file' and 'test_path_is_missing'. Nit: it might make sense to briefly mention why we do this exercise. Like, what does `test_path_is_file` et al give us over `test -f`? > diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh > index 7614dfbd95..93d6b4c479 100755 > --- a/t/t9811-git-p4-label-import.sh > +++ b/t/t9811-git-p4-label-import.sh > @@ -62,9 +62,9 @@ test_expect_success 'basic p4 labels' ' > > cd main && > git checkout TAG_F1_ONLY && > - ! test -f f2 && > + test_path_is_missing f2 && > git checkout TAG_WITH\$_SHELL_CHAR && > - test -f f1 && test -f f2 && test -f file_with_\$metachar && > + test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar && While at it we could split this line into three lines -- it's getting overly long, and we typically don't chain multiple commands on one line nowadays. > @@ -135,9 +135,9 @@ test_expect_success 'export git tags to p4' ' > p4 labels ... | grep LIGHTWEIGHT_TAG && > p4 label -o GIT_TAG_1 | grep "tag created in git:xyzzy" && > p4 sync ...@GIT_TAG_1 && > - ! test -f main/f10 && > + test_path_is_missing main/f10 && This is a stronger guarantee compared to before, as we only checked whether the path is not a file. Now we verify that it doesn't exist at all, which would be equivalent to `test -e`. That's a strict improvement though, but may be worth pointing out in the commit message so that the reviewer is not surprised. > @@ -168,9 +168,9 @@ test_expect_success 'export git tags to p4 with deletion' ' > cd "$cli" && > p4 sync ... && > p4 sync ...@GIT_TAG_ON_DELETED && > - test -f main/deleted_file && > + test_path_is_file main/deleted_file && > p4 sync ...@GIT_TAG_AFTER_DELETION && > - ! test -f main/deleted_file && > + test_path_is_missing main/deleted_file && Same here. Other than that the patch looks good to me, thanks! Patrick ``` ## Junio C Hamano, 2026-07-03 20:48 Subject: Re: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*' Message-ID: In-Reply-To: ``` Patrick Steinhardt writes: >> - test -f f1 && test -f f2 && test -f file_with_\$metachar && >> + test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar && > > While at it we could split this line into three lines -- it's getting > overly long, and we typically don't chain multiple commands on one line > nowadays. Excellent. >> - ! test -f main/f10 && >> + test_path_is_missing main/f10 && > > This is a stronger guarantee compared to before, as we only checked > whether the path is not a file. Now we verify that it doesn't exist at > all, which would be equivalent to `test -e`. That's a strict improvement > though, but may be worth pointing out in the commit message so that the > reviewer is not surprised. Good. ``` ## Marcelo Machado Lage, 2026-07-06 15:00 Subject: Re: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*' Message-ID: In-Reply-To: ``` Em sex., 3 de jul. de 2026 às 05:20, Patrick Steinhardt escreveu: > > On Thu, Jul 02, 2026 at 11:07:04AM -0300, Marcelo Machado Lage wrote: > > Replace the basic shell commands 'test -f', with more modern test > > helpers 'test_path_is_file' and 'test_path_is_missing'. > > Nit: it might make sense to briefly mention why we do this exercise. > Like, what does `test_path_is_file` et al give us over `test -f`? We'll add this in v2. > > > diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh > > index 7614dfbd95..93d6b4c479 100755 > > --- a/t/t9811-git-p4-label-import.sh > > +++ b/t/t9811-git-p4-label-import.sh > > @@ -62,9 +62,9 @@ test_expect_success 'basic p4 labels' ' > > > > cd main && > > git checkout TAG_F1_ONLY && > > - ! test -f f2 && > > + test_path_is_missing f2 && > > git checkout TAG_WITH\$_SHELL_CHAR && > > - test -f f1 && test -f f2 && test -f file_with_\$metachar && > > + test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar && > > While at it we could split this line into three lines -- it's getting > overly long, and we typically don't chain multiple commands on one line > nowadays. We'll do this for v2 as well and make it into a patch series to separate test interface modernization from formatting changes. While on this, there are some other places in the file where multiple commands in a && chain appear in a single line, e.g. in line 244: > p4 edit f2 && date >f2 && p4 submit -d "change" f2 && Should we split these into multiple lines as well, even though they are under the 80 characters limit? > > > @@ -135,9 +135,9 @@ test_expect_success 'export git tags to p4' ' > > p4 labels ... | grep LIGHTWEIGHT_TAG && > > p4 label -o GIT_TAG_1 | grep "tag created in git:xyzzy" && > > p4 sync ...@GIT_TAG_1 && > > - ! test -f main/f10 && > > + test_path_is_missing main/f10 && > > This is a stronger guarantee compared to before, as we only checked > whether the path is not a file. Now we verify that it doesn't exist at > all, which would be equivalent to `test -e`. That's a strict improvement > though, but may be worth pointing out in the commit message so that the > reviewer is not surprised. We overlooked this improvement at first, but we'll add a proper note about it in v2. > > > @@ -168,9 +168,9 @@ test_expect_success 'export git tags to p4 with deletion' ' > > cd "$cli" && > > p4 sync ... && > > p4 sync ...@GIT_TAG_ON_DELETED && > > - test -f main/deleted_file && > > + test_path_is_file main/deleted_file && > > p4 sync ...@GIT_TAG_AFTER_DELETION && > > - ! test -f main/deleted_file && > > + test_path_is_missing main/deleted_file && > > Same here. > > Other than that the patch looks good to me, thanks! Thanks for the detailed feedback, Patrick! Best, Marcelo > > Patrick ``` ## Patrick Steinhardt, 2026-07-07 14:51 Subject: Re: [PATCH] t9811: replace 'test -f' and '! test -f' with 'test_path_*' Message-ID: In-Reply-To: ``` On Mon, Jul 06, 2026 at 12:00:00PM -0300, Marcelo Machado Lage wrote: > Em sex., 3 de jul. de 2026 às 05:20, Patrick Steinhardt escreveu: > > On Thu, Jul 02, 2026 at 11:07:04AM -0300, Marcelo Machado Lage wrote: > > > diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh > > > index 7614dfbd95..93d6b4c479 100755 > > > --- a/t/t9811-git-p4-label-import.sh > > > +++ b/t/t9811-git-p4-label-import.sh > > > @@ -62,9 +62,9 @@ test_expect_success 'basic p4 labels' ' > > > > > > cd main && > > > git checkout TAG_F1_ONLY && > > > - ! test -f f2 && > > > + test_path_is_missing f2 && > > > git checkout TAG_WITH\$_SHELL_CHAR && > > > - test -f f1 && test -f f2 && test -f file_with_\$metachar && > > > + test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar && > > > > While at it we could split this line into three lines -- it's getting > > overly long, and we typically don't chain multiple commands on one line > > nowadays. > > We'll do this for v2 as well and make it into a patch series to > separate test interface modernization from formatting changes. > > While on this, there are some other places in the file where multiple > commands in a && chain appear in a single line, e.g. in line 244: > > p4 edit f2 && date >f2 && p4 submit -d "change" f2 && > Should we split these into multiple lines as well, even though they > are under the 80 characters limit? Sure, if you want to convert this into a patch series anyway then I think it makes sense to adapt all such locations in this test suite. Patrick ``` ## Marcelo Machado Lage, 2026-07-11 16:04 Subject: [PATCH v2 0/2] t9811: reformat and modernize tests Message-ID: <20260711160447.99708-1-marcelomlage@usp.br> In-Reply-To: <20260702140704.65805-1-marcelomlage@usp.br> ``` This patch series reformats and modernizes the t9811 tests. Changes since v1: - Break long && chains into multiple lines according to how git tests are written nowadays. This was suggested by Patrick Steinhardt. - Replace 'test -f' calls by more useful 'test_path_*' helpers as the second commit in the series. Marcelo Machado Lage (2): t9811: break long && chains into multiple lines t9811: replace 'test -f' and '! test -f' with 'test_path_*' t/t9811-git-p4-label-import.sh | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) Range-diff against v1: -: ---------- > 1: 0f03c913eb t9811: break long && chains into multiple lines 1: f319f2e6e7 ! 2: 3e590881c3 t9811: replace 'test -f' and '! test -f' with 'test_path_*' @@ Commit message Replace the basic shell commands 'test -f', with more modern test helpers 'test_path_is_file' and 'test_path_is_missing'. + These modern helpers emit useful information when the corresponding + tests fail, unlike 'test -f' and '! test -f'. + + The occurrences of '! test -f filename' were replaced by + 'file_path_is_missing filename', a stronger guarantee equivalent to + '! test -e filename'. + + Co-authored-by: Vinicius Lira de Freitas + Signed-off-by: Vinicius Lira de Freitas + Signed-off-by: Marcelo Machado Lage ## t/t9811-git-p4-label-import.sh ## @@ t/t9811-git-p4-label-import.sh: test_expect_success 'basic p4 labels' ' @@ t/t9811-git-p4-label-import.sh: test_expect_success 'basic p4 labels' ' - ! test -f f2 && + test_path_is_missing f2 && git checkout TAG_WITH\$_SHELL_CHAR && -- test -f f1 && test -f f2 && test -f file_with_\$metachar && -+ test_path_is_file f1 && test_path_is_file f2 && test_path_is_file file_with_\$metachar && +- test -f f1 && +- test -f f2 && +- test -f file_with_\$metachar && ++ test_path_is_file f1 && ++ test_path_is_file f2 && ++ test_path_is_file file_with_\$metachar && git show TAG_LONG_LABEL | grep -q "A Label second line" ) -- 2.34.1 ``` ## Marcelo Machado Lage, 2026-07-11 16:04 Subject: [PATCH v2 1/2] t9811: break long && chains into multiple lines Message-ID: <20260711160447.99708-2-marcelomlage@usp.br> In-Reply-To: <20260711160447.99708-1-marcelomlage@usp.br> ``` Rewrite single-line && chains by breaking them into multiple lines. Co-authored-by: Vinicius Lira de Freitas Signed-off-by: Vinicius Lira de Freitas Signed-off-by: Marcelo Machado Lage --- t/t9811-git-p4-label-import.sh | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh index 7614dfbd95..072bc88210 100755 --- a/t/t9811-git-p4-label-import.sh +++ b/t/t9811-git-p4-label-import.sh @@ -64,7 +64,9 @@ test_expect_success 'basic p4 labels' ' git checkout TAG_F1_ONLY && ! test -f f2 && git checkout TAG_WITH\$_SHELL_CHAR && - test -f f1 && test -f f2 && test -f file_with_\$metachar && + test -f f1 && + test -f f2 && + test -f file_with_\$metachar && git show TAG_LONG_LABEL | grep -q "A Label second line" ) @@ -231,17 +233,25 @@ test_expect_success 'importing labels with missing revisions' ' P4CLIENT=missing-revision && client_view "//depot/missing-revision/... //missing-revision/..." && cd "$cli" && - >f1 && p4 add f1 && p4 submit -d "start" && + >f1 && + p4 add f1 && + p4 submit -d "start" && p4 tag -l TAG_S0 ... && - >f2 && p4 add f2 && p4 submit -d "second" && + >f2 && + p4 add f2 && + p4 submit -d "second" && startrev=$(p4_head_revision //depot/missing-revision/...) && - >f3 && p4 add f3 && p4 submit -d "third" && + >f3 && + p4 add f3 && + p4 submit -d "third" && - p4 edit f2 && date >f2 && p4 submit -d "change" f2 && + p4 edit f2 && + date >f2 && + p4 submit -d "change" f2 && endrev=$(p4_head_revision //depot/missing-revision/...) && -- 2.34.1 ``` ## Marcelo Machado Lage, 2026-07-11 16:04 Subject: [PATCH v2 2/2] t9811: replace 'test -f' and '! test -f' with 'test_path_*' Message-ID: <20260711160447.99708-3-marcelomlage@usp.br> In-Reply-To: <20260711160447.99708-1-marcelomlage@usp.br> ``` Replace the basic shell commands 'test -f', with more modern test helpers 'test_path_is_file' and 'test_path_is_missing'. These modern helpers emit useful information when the corresponding tests fail, unlike 'test -f' and '! test -f'. The occurrences of '! test -f filename' were replaced by 'file_path_is_missing filename', a stronger guarantee equivalent to '! test -e filename'. Co-authored-by: Vinicius Lira de Freitas Signed-off-by: Vinicius Lira de Freitas Signed-off-by: Marcelo Machado Lage --- t/t9811-git-p4-label-import.sh | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh index 072bc88210..866d7b597b 100755 --- a/t/t9811-git-p4-label-import.sh +++ b/t/t9811-git-p4-label-import.sh @@ -62,11 +62,11 @@ test_expect_success 'basic p4 labels' ' cd main && git checkout TAG_F1_ONLY && - ! test -f f2 && + test_path_is_missing f2 && git checkout TAG_WITH\$_SHELL_CHAR && - test -f f1 && - test -f f2 && - test -f file_with_\$metachar && + test_path_is_file f1 && + test_path_is_file f2 && + test_path_is_file file_with_\$metachar && git show TAG_LONG_LABEL | grep -q "A Label second line" ) @@ -104,11 +104,11 @@ test_expect_success 'two labels on the same changelist' ' git checkout TAG_F1_1 && ls && - test -f f1 && + test_path_is_file f1 && git checkout TAG_F1_2 && ls && - test -f f1 + test_path_is_file f1 ) ' @@ -137,9 +137,9 @@ test_expect_success 'export git tags to p4' ' p4 labels ... | grep LIGHTWEIGHT_TAG && p4 label -o GIT_TAG_1 | grep "tag created in git:xyzzy" && p4 sync ...@GIT_TAG_1 && - ! test -f main/f10 && + test_path_is_missing main/f10 && p4 sync ...@GIT_TAG_2 && - test -f main/f10 + test_path_is_file main/f10 ) ' @@ -170,9 +170,9 @@ test_expect_success 'export git tags to p4 with deletion' ' cd "$cli" && p4 sync ... && p4 sync ...@GIT_TAG_ON_DELETED && - test -f main/deleted_file && + test_path_is_file main/deleted_file && p4 sync ...@GIT_TAG_AFTER_DELETION && - ! test -f main/deleted_file && + test_path_is_missing main/deleted_file && echo "checking label contents" && p4 label -o GIT_TAG_ON_DELETED | grep "tag on deleted file" ) -- 2.34.1 ``` ## Patrick Steinhardt, 2026-07-13 11:10 Subject: Re: [PATCH v2 0/2] t9811: reformat and modernize tests Message-ID: In-Reply-To: <20260711160447.99708-1-marcelomlage@usp.br> ``` On Sat, Jul 11, 2026 at 01:04:45PM -0300, Marcelo Machado Lage wrote: > This patch series reformats and modernizes the t9811 tests. > Changes since v1: > - Break long && chains into multiple lines according to how git tests are > written nowadays. This was suggested by Patrick Steinhardt. > - Replace 'test -f' calls by more useful 'test_path_*' helpers as the > second commit in the series. Thanks, I'm happy with this version! Patrick ```