On Thu, Feb 29, 2024 at 11:06:41AM -0800, Junio C Hamano wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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).
Good point. I've sent a patch in reply to your message that hopefully
clarifies this a bit. Thanks!
Patrick
Attachment:
signature.asc
Description: PGP signature
