Re: [PATCH] replace test -f with test_path_is_file

[Junio C Hamano <gitster@pobox.com>]

"Krushnal Patel via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: krush11 <krushnalpatel11@gmail.com>
>
> Although  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  with test_path_is_file so that future developers
> will have a better experience debugging these test cases.

While this change is not wrong per-se, in the context of this test
script, I think the original use of "test -f" is not quite right to
begin with.  These are all "even after running 'git clean', these
paths should exist without getting removed by mistake", so the
intent of these "test -f" invocations are actually "test -e".

Similarly, the invocations of "test ! -f" we see (and there also is
at least one "! test -d") mean to say "these paths should be gone as
the result of running 'git clean'".  If by some accident a directory
exists at the path that is checked with "test ! -f" due to a bug in
'git clean', these tests will not catch such a bug, because a directory
does not pass "test -f".

So most likely these negative tests this patch does not convert are
better off being spelled as "! test -e", too.

It would be more appropriate to use test_path_exists and
test_path_is_missing to replace these "must exist as a file" and
"must not exist as a file".

Thanks.