Re: [PATCH 1/1] [GSoC][PATCH] t3070: refactor test -e command

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 29, 2024, 19:06 UTC
Message-ID: <xmqqzfvjf5tq.fsf@gitster.g>
In-Reply-To: <CAPig+cR2-6qONkosu7=qEQSJa_fvYuVQ0to47D5qx904zW08Eg@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 16 quoted lines

>> @@ -175,7 +175,7 @@ match() {
>>         test_expect_success EXPENSIVE_ON_WINDOWS 'cleanup after previous file test' '
>> -               if test -e .git/created_test_file
>> +               if test_path_exists .git/created_test_file
>>                 then
>>                         git reset &&
>
> ... which _do_ use test_path_exists() within a `test_expect_success`
> block. However, the changes are still undesirable because, as above,
> this `test -e` is merely part of the normal control-flow; it's not
> acting as an assertion, thus test_path_exists() -- which is an
> assertion -- is not correct.
>
> Unfortunately, none of the uses of`test -e` in t3070 are being used as
> assertions worthy of replacement with test_path_exists(), thus this
> isn't a good script in which to make such changes.

It seems that there is a recurring confusion among mentorship program applicants that use test_path_* helpers as their practice material. Perhaps the source of the information that suggests it as a microproject is poorly phrased and needs to be rewritten to avoid misleading them.

I found one at https://git.github.io/Outreachy-23-Microprojects/, which can be one source of such confusion:

    Find one test script that verifies the presence/absence of
    files/directories with ‘test -(e|f|d|…)’ and replace them
    with the appropriate test_path_is_file, test_path_is_dir,
    etc. helper functions.

but there may be others.

This task specification does not differenciate "test -[efdx]" used as a conditional of a control flow statement (which should never be replaced by test_path_* helpers) and those used to directly fail the &&-chain in test_expect_success with their exit status (which is the target that test_path_* helpers are meant to improve).