| Message ID | 542cd04c2ebda88b8fa63dc4dcb1c42d10afc844.1571355109.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Thu, Oct 17, 2019 at 7:35 PM Denton Liu <liu.denton@gmail.com> wrote: > Although `test -f` has the same functionality as test_path_is_file(), in > the case where test_path_is_file() fails, we get much better debugging > information. > > Replace `test -f` with test_path_is_file() so that future developers > will have a better experience debugging these test cases. Also, in the > case of `! test -f`, replace it with test_path_is_missing(). > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > I just realised that test_path_is_missing() is a much better replacement > than `test_must_fail test_path_is_file`. That depends upon context... > diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh > @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' > - ! test -f file > + test_path_is_missing file There is a semantic difference between checking that "file" is not a _file_ versus checking that the a path itself (which may be a directory or a "special file") does not exist. In this particular test, the difference doesn't matter, so the conversion is sensible enough, but it would save reviewers time if you, as author of the patch, state that you carefully considered the distinction before making each change.
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 55560ce3cd..004d5884cd 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -39,8 +39,8 @@ test_expect_success 'pulling into void' ' cd cloned && git pull .. ) && - test -f file && - test -f cloned/file && + test_path_is_file file && + test_path_is_file cloned/file && test_cmp file cloned/file ' @@ -50,8 +50,8 @@ test_expect_success 'pulling into void using master:master' ' cd cloned-uho && git pull .. master:master ) && - test -f file && - test -f cloned-uho/file && + test_path_is_file file && + test_path_is_file cloned-uho/file && test_cmp file cloned-uho/file ' @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' ( cd cloned-octopus && test_must_fail git pull .. master master && - ! test -f file + test_path_is_missing file ) '
Although `test -f` has the same functionality as test_path_is_file(), in the case where test_path_is_file() fails, we get much better debugging information. Replace `test -f` with test_path_is_file() so that future developers will have a better experience debugging these test cases. Also, in the case of `! test -f`, replace it with test_path_is_missing(). Signed-off-by: Denton Liu <liu.denton@gmail.com> --- I just realised that test_path_is_missing() is a much better replacement than `test_must_fail test_path_is_file`. Please use this patch instead of 04/12. t/t5520-pull.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)