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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 15, 2022, 22:13 UTC
Message-ID: <220215.86a6erwzee.gmgdl@evledraar.gmail.com>
In-Reply-To: <6fbd4188-3bb5-6d48-fd25-1bdbe9a3cbfb@gmail.com>

On Mon, Feb 14 2022, Cogoni Guillaume wrote:

Show 12 quoted lines

>>   * 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.