ssh test: make copy_ssh_wrapper_as clean up after itself

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> Acked-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Nieder committed Nov 20, 2017 at 17:49 UTC 8339805b467ca5b2d9314fdbfdd75a6e96c6b39a
1 file changed +12 -16
t/t5601-clone.sh
+12 -16
@@ -306,21 +306,20 @@ test_expect_success 'clone checking out a tag' '
306 test_cmp fetch.expected fetch.actual
307 '
308
309 -setup_ssh_wrapper () {
310 - test_expect_success 'setup ssh wrapper' '
311 - cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
312 - "$TRASH_DIRECTORY/ssh$X" &&
313 - GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
314 - export GIT_SSH &&
315 - export TRASH_DIRECTORY &&
316 - >"$TRASH_DIRECTORY"/ssh-output
317 - '
318 -}
309 +test_expect_success 'set up ssh wrapper' '
310 + cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
311 + "$TRASH_DIRECTORY/ssh$X" &&
312 + GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
313 + export GIT_SSH &&
314 + export TRASH_DIRECTORY &&
315 + >"$TRASH_DIRECTORY"/ssh-output
316 +'
317
318 copy_ssh_wrapper_as () {
319 cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" &&
320 + test_when_finished "rm $(git rev-parse --sq-quote "${1%$X}$X")" &&
321 GIT_SSH="${1%$X}$X" &&
323 - export GIT_SSH
322 + test_when_finished "GIT_SSH=\"\$TRASH_DIRECTORY/ssh\$X\""
323 }
324
325 expect_ssh () {
@@ -344,8 +343,6 @@ expect_ssh () {
343 (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
344 }
345
347 -setup_ssh_wrapper
348 -
346 test_expect_success 'clone myhost:src uses ssh' '
347 git clone myhost:src ssh-clone &&
348 expect_ssh myhost src
@@ -432,12 +429,14 @@ test_expect_success 'ssh.variant overrides plink detection' '
429 '
430
431 test_expect_success 'GIT_SSH_VARIANT overrides plink detection to plink' '
432 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
433 GIT_SSH_VARIANT=plink \
434 git clone "[myhost:123]:src" ssh-bracket-clone-variant-3 &&
435 expect_ssh "-P 123" myhost src
436 '
437
438 test_expect_success 'GIT_SSH_VARIANT overrides plink to tortoiseplink' '
439 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
440 GIT_SSH_VARIANT=tortoiseplink \
441 git clone "[myhost:123]:src" ssh-bracket-clone-variant-4 &&
442 expect_ssh "-batch -P 123" myhost src
@@ -449,9 +448,6 @@ test_expect_success 'clean failure on broken quoting' '
448 git clone "[myhost:123]:src" sq-failure
449 '
450
452 -# Reset the GIT_SSH environment variable for clone tests.
453 -setup_ssh_wrapper
454 -
451 counter=0
452 # $1 url
453 # $2 none|host