git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/8] ssh test: make copy_ssh_wrapper_as clean up after itself

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Nov 20, 2017, 21:22 UTC
Message-ID
<20171120212223.wquyxbmz34foynrk@aiede.mtv.corp.google.com>
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
Previous: Jonathan NiederNext: Brandon Williams
Message 2 of 27 in “Coping with unrecognized ssh wrapper scripts in GIT_SSH”
  1. 0/8 Coping with unrecognized ssh wrapper scripts in GIT_SSHJonathan Nieder, Nov 20, 2017
  2. 1/8 ssh test: make copy_ssh_wrapper_as clean up after itselfJonathan Nieder, Nov 20, 2017
  3. Brandon WilliamsNov 20, 2017
  4. Junio C HamanoNov 21, 2017
  5. 1/8 ssh test: make copy_ssh_wrapper_as clean up after itselfJonathan Nieder, Nov 21, 2017
  6. Stefan BellerNov 21, 2017
  7. 2/8 connect: move no_fork fallback to git_tcp_connectJonathan Nieder, Nov 20, 2017
  8. 3/8 connect: split git:// setup into a separate functionJonathan Nieder, Nov 20, 2017
  9. Brandon WilliamsNov 20, 2017
  10. Jonathan NiederNov 20, 2017
  11. Brandon WilliamsNov 20, 2017
  12. 4/8 connect: split ssh command line options into separate functionJonathan Nieder, Nov 20, 2017
  13. Brandon WilliamsNov 20, 2017
  14. Jonathan NiederNov 20, 2017
  15. Brandon WilliamsNov 20, 2017
  16. 4/8 connect: split ssh command line options into separate functionJonathan Nieder, Nov 20, 2017
  17. 5/8 connect: split ssh option computation to its own functionJonathan Nieder, Nov 20, 2017
  18. Junio C HamanoNov 21, 2017
  19. 6/8 ssh: 'auto' variant to select between 'ssh' and 'simple'Jonathan Nieder, Nov 20, 2017
  20. Brandon WilliamsNov 20, 2017
  21. Junio C HamanoNov 21, 2017
  22. Jonathan NiederNov 21, 2017
  23. 7/8 ssh: 'simple' variant does not support -4/-6Jonathan Nieder, Nov 20, 2017
  24. 8/8 ssh: 'simple' variant does not support --portJonathan Nieder, Nov 20, 2017
  25. Brandon WilliamsNov 20, 2017
  26. Stefan BellerNov 22, 2017
  27. Junio C HamanoNov 22, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.