All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: shejialuo <shejialuo@gmail.com>,  git@vger.kernel.org
Subject: Re: [PATCH 1/1] [GSoC][PATCH] t3070: refactor test -e command
Date: Thu, 29 Feb 2024 11:06:41 -0800	[thread overview]
Message-ID: <xmqqzfvjf5tq.fsf@gitster.g> (raw)
In-Reply-To: <CAPig+cR2-6qONkosu7=qEQSJa_fvYuVQ0to47D5qx904zW08Eg@mail.gmail.com> (Eric Sunshine's message of "Thu, 29 Feb 2024 12:58:03 -0500")

Eric Sunshine <sunshine@sunshineco.com> writes:

>> @@ -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).

  reply	other threads:[~2024-02-29 19:06 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-29 15:04 [GSoC][PATCH 0/1] microproject: Use test_path_is_* functions in test scripts shejialuo
2024-02-29 15:04 ` [PATCH 1/1] [GSoC][PATCH] t3070: refactor test -e command shejialuo
2024-02-29 17:58   ` Eric Sunshine
2024-02-29 19:06     ` Junio C Hamano [this message]
2024-03-04  9:16       ` [PATCH] SoC 2024: clarify `test_path_is_*` conversion microproject Patrick Steinhardt
2024-03-04 13:42         ` Christian Couder
2024-03-04 17:02         ` Junio C Hamano
2024-03-04  9:17       ` [PATCH 1/1] [GSoC][PATCH] t3070: refactor test -e command Patrick Steinhardt
2024-03-01  2:50     ` shejialuo
2024-03-01  3:46 ` [PATCH V2 0/1] [GSoC][PATCH] t9117: prefer test_path_* helper functions shejialuo
2024-03-01  3:46   ` [PATCH 1/1] " shejialuo
2024-03-01  4:44     ` Eric Sunshine
2024-03-01 11:29       ` shejialuo
2024-03-01  5:09     ` Junio C Hamano
2024-03-01 11:36       ` shejialuo
2024-03-01 13:03   ` [PATCH v3 0/1] " shejialuo
2024-03-01 13:03     ` [PATCH v3 1/1] [PATCH] " shejialuo
2024-03-04  9:24       ` Patrick Steinhardt
2024-03-04  9:54     ` [PATCH v4 0/1] Change commit message shejialuo
2024-03-04  9:54       ` [PATCH v4 1/1] [PATCH] t9117: prefer test_path_* helper functions shejialuo
2024-03-04  9:59         ` Patrick Steinhardt
2024-03-04 11:45           ` shejialuo
2024-03-04 17:50           ` Junio C Hamano
2024-03-04 17:22         ` Junio C Hamano
2024-03-05 11:42           ` shejialuo
2024-03-04 17:19       ` [PATCH v4 0/1] Change commit message Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzfvjf5tq.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=shejialuo@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.