"Krushnal Patel via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: krush11 <krushnalpatel11@xxxxxxxxx> > > 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.
