From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: PRASHANT S BISHT <prashantjee2025@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] t0004: replace test -e with test_path_exists
Date: Mon, 9 Mar 2026 18:47:39 -0400 [thread overview]
Message-ID: <20260309224739.GA5682@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqq4imo4sf1.fsf@gitster.g>
On Mon, Mar 09, 2026 at 02:14:10PM -0700, Junio C Hamano wrote:
> PRASHANT S BISHT <prashantjee2025@gmail.com> writes:
>
> > -test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
> > +test_lazy_prereq WRITE_TREE_OUT 'test_path_exists "$TRASH_DIRECTORY/out.write-tree"'
>
> I suspect this is utterly wrong. As you wrote in the proposed log
> message, test_path_exists is *NOT* about checking if the path
> exists. It rather is about *expecting* for the path to exist, and
> fail *LOUDLY* if it does not.
>
> You need to _think_ if we want a LOUD failure when somebody checks
> if a path exists and conditionally skip setting a test prerequisite
> when the path does not exist. The original code is trying to be
> quiet, as the check is done not because existence of the checked
> path is good and lack of it is a test failure. Lack of the path is
> expected on places where the prerequisite is not set, and that by
> itself is not a test failure that you want a LOUD report about.
I'm not sure I agree. Verbose prereq blocks can help with debugging.
Normally you would not see them at all, but if you are investigating why
a prereq did not trigger, you may want more output.
Without "-v" you would not see the output either way, like:
ok 1 # skip some test (missing FOO)
But with it, it is the difference between:
checking prerequisite: FOO
mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
(
cd "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
test -e foo
)
prerequisite FOO not satisfied
ok 1 # skip some test (missing FOO)
and:
checking prerequisite: FOO
mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
(
cd "$TRASH_DIRECTORY/prereq-test-dir-FOO" &&
test_path_exists foo
)
Path foo doesn't exist
prerequisite FOO not satisfied
ok 1 # skip some test (missing FOO)
Probably it's pretty obvious for a one-liner like this, but I think it
would help for a longer block.
-Peff
next prev parent reply other threads:[~2026-03-09 22:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-09 17:36 [PATCH] t0004: replace test -e with test_path_exists PRASHANT S BISHT
2026-03-09 21:14 ` Junio C Hamano
2026-03-09 22:47 ` Jeff King [this message]
2026-03-09 23:12 ` Junio C Hamano
2026-03-16 17:24 ` [PATCH v2] t4200: convert test -[df] checks to test_path_* helpers PRASHANT S BISHT
-- strict thread matches above, loose matches on Subject: below --
2026-03-09 17:56 [PATCH] t0004: replace test -e with test_path_exists PRASHANT S BISHT
2026-03-09 19:30 ` Eric Sunshine
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=20260309224739.GA5682@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=prashantjee2025@gmail.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.