Subject: Re: [PATCH] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 15 Feb 2022 23:13:11 +0100

On Mon, Feb 14 2022, Cogoni Guillaume wrote:

>>   * verify that all existing users of test_path_is_file want to
>>     reject a symlink to file, and add 'test ! -h "$1" &&' to the
>>     implementation of the test helper in t/test-lib-functions.sh
>>     (we may want to do the same for test_path_is_dir).
>>
>>   * introduce test_path_is_symlink and use it appropriately.  This
>>     will be a more verbose version of "test -h".
>>
>>   * introduce test_path_is_file_not_symlink and use it here.
>
> We wouldn't modify test_path_is_file because this function is already
> use and we won't verify if every uses of this are rejecting symlink.

Perhaps it's not a good idea (I haven't checked) to change it like that.

But it's fine to change these sorts of test functions even if there's
existing users of it, our test suite isn't a stable API.

Of course one still has to consider outstanding patches, anything in the
list archive we may want to dig up etc., so it's best not to do so
without good reason.

But the "verifying every use" should mostly be just running "make test",
and pushing to the GitHub CI.