Re: [GSoC PATCH] t9400: prefer test_path_* helper functions

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

Aryan Pathania <contact@aynp.dev> writes:

>>This isn't quite equivalent: we've been checking that the path is not a
>>directory before, but now we verify that the path doesn't exist.
> I understand. I could not find `test_path_is_not_dir` or any equivalent
> function in `test-lib-functions.sh`. Maybe we can keep this stronger
> check. I'll mention in the commit message of next version of patch.

That is exactly Patrick suggested (go back and read it).  I agree
with him that the updated stronger check is an improvement and it
deserves to be explained in the commit message.

>>We tend to use `test_path_is_file` rather than
>>`test_path_is_file_not_symlink`, but I don't mind it too much.
> I believe `test -f` is equivalent to `test_path_is_file_not_symlink` and
> is a stronger check so maybe it's fine.

Don't believe what you think you know; if you are unsure, check to
verify before you base your actions on them.

    $ >this-is-file
    $ ln -s this-is-file this-is-symlink
    $ test -f this-is-symlink; echo $?
    0
    $ test -f this-is-file; echo $?
    0

And if you are not unsure, then learn to be unsure more often ;-)

I do not see any reason in the part of the code Patrick commented on
to insist that gitcvs.ext.main.sqlite file must be a regular file
and not a symbolic link to another file.  Both test_path_is_file
and its original before the patch, "test -f", would be more appropriate
than test_path_is_file_not_symlink, which was specifically invented
for use in t3903 where the tests used both files and symbolic links
to make sure the operation being tested would not confuse one with
the other.

> Sorry for the trouble and mistakes.

No need for that.  This is for both sides to experience to learn to
work well together.