Thread (1 message) 1 message, 1 author, 2021-03-19

Re: [PATCH] replace test -f with test_path_is_file

From: Junio C Hamano <hidden>
Date: 2021-03-19 18:05:11

"Krushnal Patel via GitGitGadget" [off-list ref] writes:
From: krush11 <redacted>

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help