git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Brandon Williams <bmwill@google.com>,
	Stefan Beller <sbeller@google.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Segev Finer <segev208@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 1/8] ssh test: make copy_ssh_wrapper_as clean up after itself
Date: Mon, 20 Nov 2017 13:22:23 -0800	[thread overview]
Message-ID: <20171120212223.wquyxbmz34foynrk@aiede.mtv.corp.google.com> (raw)
In-Reply-To: <20171120212134.lh2l4drdzu6fh5g2@aiede.mtv.corp.google.com>

Simplify by not allowing the copied ssh wrapper to persist between
tests.  This way, tests can be safely reordered, added, and removed
with less fear of hidden side effects.

This also avoids having to call setup_ssh_wrapper to restore the value
of GIT_SSH after this battery of tests, since it means each test will
restore it individually.

Noticed because on Windows, if `uplink.exe` exists, the MSYS2 Bash
will overwrite that when redirecting via `>uplink`.  A proposed test
wrote a script to 'uplink' after a previous test created uplink.exe
using copy_ssh_wrapper_as, so the script written with '>uplink' had
the wrong filename and failed.

Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks to Dscho for tracking this subtle issue down.

 t/t5601-clone.sh | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 86811a0c35..9d007c0f8d 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -306,21 +306,20 @@ test_expect_success 'clone checking out a tag' '
 	test_cmp fetch.expected fetch.actual
 '
 
-setup_ssh_wrapper () {
-	test_expect_success 'setup ssh wrapper' '
-		cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
-			"$TRASH_DIRECTORY/ssh$X" &&
-		GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
-		export GIT_SSH &&
-		export TRASH_DIRECTORY &&
-		>"$TRASH_DIRECTORY"/ssh-output
-	'
-}
+test_expect_success 'set up ssh wrapper' '
+	cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
+		"$TRASH_DIRECTORY/ssh$X" &&
+	GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
+	export GIT_SSH &&
+	export TRASH_DIRECTORY &&
+	>"$TRASH_DIRECTORY"/ssh-output
+'
 
 copy_ssh_wrapper_as () {
 	cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" &&
+	test_when_finished "rm -f ${1%$X}$X" &&
 	GIT_SSH="${1%$X}$X" &&
-	export GIT_SSH
+	test_when_finished "GIT_SSH=\"\$TRASH_DIRECTORY/ssh\$X\""
 }
 
 expect_ssh () {
@@ -344,8 +343,6 @@ expect_ssh () {
 	(cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
 }
 
-setup_ssh_wrapper
-
 test_expect_success 'clone myhost:src uses ssh' '
 	git clone myhost:src ssh-clone &&
 	expect_ssh myhost src
@@ -432,12 +429,14 @@ test_expect_success 'ssh.variant overrides plink detection' '
 '
 
 test_expect_success 'GIT_SSH_VARIANT overrides plink detection to plink' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
 	GIT_SSH_VARIANT=plink \
 	git clone "[myhost:123]:src" ssh-bracket-clone-variant-3 &&
 	expect_ssh "-P 123" myhost src
 '
 
 test_expect_success 'GIT_SSH_VARIANT overrides plink to tortoiseplink' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
 	GIT_SSH_VARIANT=tortoiseplink \
 	git clone "[myhost:123]:src" ssh-bracket-clone-variant-4 &&
 	expect_ssh "-batch -P 123" myhost src
@@ -449,9 +448,6 @@ test_expect_success 'clean failure on broken quoting' '
 		git clone "[myhost:123]:src" sq-failure
 '
 
-# Reset the GIT_SSH environment variable for clone tests.
-setup_ssh_wrapper
-
 counter=0
 # $1 url
 # $2 none|host
-- 
2.15.0.448.gf294e3d99a


  reply	other threads:[~2017-11-20 21:22 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-20 21:21 [PATCH v3 0/8] Coping with unrecognized ssh wrapper scripts in GIT_SSH Jonathan Nieder
2017-11-20 21:22 ` Jonathan Nieder [this message]
2017-11-20 21:47   ` [PATCH 1/8] ssh test: make copy_ssh_wrapper_as clean up after itself Brandon Williams
2017-11-21  1:24   ` Junio C Hamano
2017-11-21  1:49   ` [PATCH 1/8 v2] " Jonathan Nieder
2017-11-21 23:42     ` Stefan Beller
2017-11-20 21:22 ` [PATCH 2/8] connect: move no_fork fallback to git_tcp_connect Jonathan Nieder
2017-11-20 21:23 ` [PATCH 3/8] connect: split git:// setup into a separate function Jonathan Nieder
2017-11-20 21:52   ` Brandon Williams
2017-11-20 22:04     ` Jonathan Nieder
2017-11-20 22:29       ` Brandon Williams
2017-11-20 21:25 ` [PATCH 4/8] connect: split ssh command line options into " Jonathan Nieder
2017-11-20 21:54   ` Brandon Williams
2017-11-20 22:09     ` Jonathan Nieder
2017-11-20 22:28       ` Brandon Williams
2017-11-20 22:19     ` [PATCH v4 " Jonathan Nieder
2017-11-20 21:26 ` [PATCH 5/8] connect: split ssh option computation to its own function Jonathan Nieder
2017-11-21  1:31   ` Junio C Hamano
2017-11-20 21:30 ` [PATCH 6/8] ssh: 'auto' variant to select between 'ssh' and 'simple' Jonathan Nieder
2017-11-20 22:25   ` Brandon Williams
2017-11-21  1:48   ` Junio C Hamano
2017-11-21  2:01     ` Jonathan Nieder
2017-11-20 21:30 ` [PATCH 7/8] ssh: 'simple' variant does not support -4/-6 Jonathan Nieder
2017-11-20 21:31 ` [PATCH 8/8] ssh: 'simple' variant does not support --port Jonathan Nieder
2017-11-20 22:32 ` [PATCH v3 0/8] Coping with unrecognized ssh wrapper scripts in GIT_SSH Brandon Williams
2017-11-22  0:00   ` Stefan Beller
2017-11-22  1:52 ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20171120212223.wquyxbmz34foynrk@aiede.mtv.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=jonathantanmy@google.com \
    --cc=sbeller@google.com \
    --cc=segev208@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).