From: Junio C Hamano Date: Fri, 19 Mar 2021 18:04:28 GMT Subject: Re: [PATCH] replace test -f with test_path_is_file Message-ID: In-Reply-To: "Krushnal Patel via GitGitGadget" writes: > From: krush11 > > 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.