Re: [PATCH] clone: local URLs are not for ssh

[Jeff King <peff@peff.net>]

On Thu, Oct 03, 2013 at 08:01:23AM +0700, Nguyen Thai Ngoc Duy wrote:

> > Sorry for the noise, I noticed it when I was trying to construct test cases.
> >
> > What do we think about adding this at the end of t5505:
> 
> As usual more tests are usually better. But is t5505-remote.sh the
> best place? That file seems about "git remote"..

Yeah, agreed. How about at the end of t5601, after the ssh wrapper I set
up here:

  http://article.gmane.org/gmane.comp.version-control.git/235473

I don't know of Jonathan squashed those in to your commit...neither is
in his 'pu' yet.

> > test_expect_success 'fetch fail [noexistinghost0:2223]:blink.git' '
> >         (
> >                 ! git fetch [noexistinghost0:2223]:blink.git 2>err &&
> >                 grep ssh err &&
> >                 rm err
> >         )
> > '

This one looks like basically the same test I added in the message
above (except because of the ssh wrapper, we can check that it did
indeed try to ssh to noexistinghost0:2223).

The other tests can check that we fed ssh various host/port/path
combinations. I'm not clear on what we're expecting, though...

> > test_expect_success 'fetch fail noexistinghost1:2223:blink.git' '
> >         (
> >                 ! git fetch "noexistinghost1:2223:blink.git" 2>err &&
> >                 grep ssh err &&
> >                 rm err
> >         )
> > '

We are expecting this to be host=noexistinghost1, and
path=2223:blink.git?

> > test_expect_success 'fetch fail noexistinghost2:2223' '
> >         (
> >                 ! git fetch "noexistinghost2:2223" 2>err &&
> >                 grep ssh err &&
> >                 rm err
> >         )
> > '

And this is host=noexistinghost2, path=2223?

> > test_expect_success 'fetch fail ./noexistinghost4:2223"' '
> >         (
> >                 ! git fetch "./noexistinghost4:2223" 2>err &&
> >                 grep "does not appear to be a git repository" err &&
> >                 rm err
> >         )
> > '

And this one we would be checking that ssh is _not_ used. It seems
redundant with the "./foo:bar" test already in t5601, but perhaps it is
worth double-checking the numeric path. It would be more robust if we
actually had a repo called "noexistinghost4:2223" and checked that we
did clone it, as the existing test does (maybe that test can just
"s/bar/2223/" ?).

-Peff