On 11/20, Jonathan Nieder wrote:
> 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@xxxxxx>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> ---
> 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\""
All the escaping!
Patch looks good.
> }
>
> 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
>
--
Brandon Williams
