git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "H.Merijn Brand" <h.m.brand@xs4all.nl>, Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Do not fail test if '.' is part of $PATH
Date: Mon, 03 Dec 2018 10:00:50 +0900	[thread overview]
Message-ID: <xmqqr2ezh1a5.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20181201180757.0b2d3c89@pc09.procura.nl> (H. Merijn Brand's message of "Sat, 1 Dec 2018 18:07:57 +0100")

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> When $PATH contains the current directory as .:PATH, PATH:., PATH:.:PATH,
> or (maybe worse) as :PATH, PATH:, or PATH::PATH - as an empty entry is
> identical to having dot in $PATH - this test used to fail

It is totally unclear what "this test" refers to.  Let's retitle it
to

> Subject: [PATCH] t0061: do not fail test if '.' is part of $PATH

and do something like this:

    t0061 created a script named with an unlikely name in the
    current directory to ensure that it is not found via the
    run_command() API, expecting that $PATH does not contain an
    element that names the current directory (i.e. '.' or '') in a
    sane environment.  This obviously would not work if the $PATH
    does contain such an element.

    Introduce a DOT_IN_PATH lazy prerequisite to catch such a case
    and skip the test when the environment is not so sane.

> +test_lazy_prereq DOT_IN_PATH '
> +       case ":$PATH:" in
> +       *:.:*|*::*) true  ;;
> +       *)          false ;;
> +       esac
> +'
> +
> +test_expect_success !DOT_IN_PATH 'run_command is restricted to PATH' '
>         write_script should-not-run <<-\EOF &&
>         echo yikes
>         EOF

I also like Peff's more straight-forward approach that avoids
looking into PATH but instead ask the shell what we care about
(i.e. would we end up running 'should-not-run' if we asked the
system to run it without giving an explicit path to it?).  The last
paragraph of the above would need to change if we were to go in that
direction to something like

    Check if the running shell picks up the script without an
    explicit path to it and skip the test when it does.

perhaps.  The code to do so got a bit more compact than what Peff
wrote but I think it still retains its main beauty, which is how
straight-forward it is.

 t/t0061-run-command.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index cf932c8514..17b560370e 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -29,7 +29,15 @@ test_expect_success 'run_command can run a command' '
 	test_must_be_empty err
 '
 
-test_expect_success 'run_command is restricted to PATH' '
+
+test_lazy_prereq RUNS_COMMANDS_FROM_PWD '
+	write_script runs-commands-from-pwd <<-\EOF &&
+	true
+	EOF
+	runs-commands-from-pwd >/dev/null 2>&1
+'
+
+test_expect_success !RUNS_COMMANDS_FROM_PWD 'run_command is restricted to PATH' '
 	write_script should-not-run <<-\EOF &&
 	echo yikes
 	EOF

      parent reply	other threads:[~2018-12-03  1:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-01 17:07 [PATCH] Do not fail test if '.' is part of $PATH H.Merijn Brand
2018-12-01 19:38 ` Jeff King
2018-12-03  0:29   ` Junio C Hamano
2018-12-03  1:00 ` Junio C Hamano [this message]

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=xmqqr2ezh1a5.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=h.m.brand@xs4all.nl \
    --cc=peff@peff.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).