On Thu, Oct 17, 2019 at 7:35 PM Denton Liu <liu.den...@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.den...@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.

Reply via email to