clients/ssh: test fix

8 files changed, 91 insertions, 162 deletions

diff --git a/.travis.yml b/.travis.yml
index b0f8e87..043d7da 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,13 +1,31 @@
 language: go
+go:
+- 1.6
+- 1.7
+- tip
+matrix:
+  allow_failures:
+  - go: tip
+
+
+language: go
 
 go:
   - 1.6
+  - 1.7
   - tip
 
 matrix:
   allow_failures:
     - go: tip
 
+
+before_install:
+  - openssl aes-256-cbc -K $encrypted_1477e58fe67a_key -iv $encrypted_1477e58fe67a_iv -in .travis/deploy.pem.enc -out .travis/deploy.pem -d
+  - eval "$(ssh-agent -s)"
+  - chmod 600 .travis/deploy.pem
+  - ssh-add .travis/deploy.pem
+
 install:
   - rm -rf $GOPATH/src/gopkg.in/src-d
   - mkdir -p $GOPATH/src/gopkg.in/src-d
diff --git a/.travis/deploy.pem.enc b/.travis/deploy.pem.enc
new file mode 100644
index 0000000..0584013
--- /dev/null
+++ b/.travis/deploy.pem.enc
diff --git a/clients/ssh/auth_method.go b/clients/ssh/auth_method.go
index 4fe68ca..e55283e 100644
--- a/clients/ssh/auth_method.go
+++ b/clients/ssh/auth_method.go
@@ -2,8 +2,11 @@ package ssh
 
 import (
 	"fmt"
+	"net"
+	"os"
 
 	"golang.org/x/crypto/ssh"
+	"golang.org/x/crypto/ssh/agent"
 	"gopkg.in/src-d/go-git.v4/clients/common"
 )
 
@@ -134,3 +137,17 @@ func (a *PublicKeysCallback) clientConfig() *ssh.ClientConfig {
 		Auth: []ssh.AuthMethod{ssh.PublicKeysCallback(a.Callback)},
 	}
 }
+
+// Opens a pipe with the ssh agent and uses the pipe
+// as the implementer of the public key callback function.
+func NewSSHAgentAuth() (*PublicKeysCallback, error) {
+	pipe, err := net.Dial("unix", os.Getenv("SSH_AUTH_SOCK"))
+	if err != nil {
+		return nil, err
+	}
+
+	return &PublicKeysCallback{
+		User:     "git",
+		Callback: agent.NewClient(pipe).Signers,
+	}, nil
+}
diff --git a/clients/ssh/git_upload_pack.go b/clients/ssh/git_upload_pack.go
index 551ab9c..03b289a 100644
--- a/clients/ssh/git_upload_pack.go
+++ b/clients/ssh/git_upload_pack.go
@@ -42,15 +42,19 @@ type GitUploadPackService struct {
 }
 
 // NewGitUploadPackService initialises a GitUploadPackService.
-// TODO: remove this, as the struct is zero-value safe.
 func NewGitUploadPackService(endpoint common.Endpoint) common.GitUploadPackService {
 	return &GitUploadPackService{endpoint: endpoint}
 }
 
 // Connect cannot be used with SSH clients and always return
 // ErrAuthRequired. Use ConnectWithAuth instead.
-func (s *GitUploadPackService) Connect() (err error) {
-	return ErrAuthRequired
+func (s *GitUploadPackService) Connect() error {
+	auth, err := NewSSHAgentAuth()
+	if err != nil {
+		return err
+	}
+
+	return s.ConnectWithAuth(auth)
 }
 
 // ConnectWithAuth connects to ep using SSH. Authentication is handled
diff --git a/clients/ssh/git_upload_pack_test.go b/clients/ssh/git_upload_pack_test.go
index 3a0a983..b26276d 100644
--- a/clients/ssh/git_upload_pack_test.go
+++ b/clients/ssh/git_upload_pack_test.go
@@ -1,97 +1,30 @@
-// +build ssh
-
 package ssh
 
 import (
-	"fmt"
 	"io/ioutil"
-	"net"
 	"os"
 
-	"golang.org/x/crypto/ssh/agent"
-
 	. "gopkg.in/check.v1"
 	"gopkg.in/src-d/go-git.v4/clients/common"
 	"gopkg.in/src-d/go-git.v4/core"
 )
 
-type SuiteRemote struct{}
-
-var _ = Suite(&SuiteRemote{})
-
-const (
-	fixRepo             = "git@github.com:tyba/git-fixture.git"
-	fixRepoBadVcs       = "www.example.com"
-	fixRepoNonGit       = "https://code.google.com/p/go"
-	fixGitRepoNonGithub = "https://bitbucket.org/user/repo.git"
-)
-
-func (s *SuiteRemote) TestConnect(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.Connect(fixRepo), Equals, ErrAuthRequired)
-}
-
-// We will use a running ssh agent for testing
-// ssh authentication.
-type sshAgentConn struct {
-	pipe net.Conn
-	auth *PublicKeysCallback
+type RemoteSuite struct {
+	Endpoint common.Endpoint
 }
 
-// Opens a pipe with the ssh agent and uses the pipe
-// as the implementer of the public key callback function.
-func newSSHAgentConn() (*sshAgentConn, error) {
-	pipe, err := net.Dial("unix", os.Getenv("SSH_AUTH_SOCK"))
-	if err != nil {
-		return nil, err
-	}
-	return &sshAgentConn{
-		pipe: pipe,
-		auth: &PublicKeysCallback{
-			User:     "git",
-			Callback: agent.NewClient(pipe).Signers,
-		},
-	}, nil
-}
+var _ = Suite(&RemoteSuite{})
 
-// Closes the pipe with the ssh agent
-func (c *sshAgentConn) close() error {
-	return c.pipe.Close()
-}
+func (s *RemoteSuite) SetUpSuite(c *C) {
+	var err error
+	s.Endpoint, err = common.NewEndpoint("git@github.com:git-fixtures/basic.git")
+	c.Assert(err, IsNil)
 
-func (s *SuiteRemote) SetUpSuite(c *C) {
 	if os.Getenv("SSH_AUTH_SOCK") == "" {
 		c.Skip("SSH_AUTH_SOCK is not set")
 	}
 }
 
-func (s *SuiteRemote) TestConnectWithPublicKeysCallback(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
-	defer func() { c.Assert(r.Disconnect(), IsNil) }()
-	c.Assert(r.connected, Equals, true)
-	c.Assert(r.auth, Equals, agent.auth)
-}
-
-func (s *SuiteRemote) TestConnectBadVcs(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepoBadVcs, nil), ErrorMatches, fmt.Sprintf(".*%s.*", fixRepoBadVcs))
-}
-
-func (s *SuiteRemote) TestConnectNonGit(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepoNonGit, nil), Equals, ErrUnsupportedVCS)
-}
-
-func (s *SuiteRemote) TestConnectNonGithub(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixGitRepoNonGithub, nil), Equals, ErrUnsupportedRepo)
-}
-
 // A mock implementation of client.common.AuthMethod
 // to test non ssh auth method detection.
 type mockAuth struct{}
@@ -99,85 +32,60 @@ type mockAuth struct{}
 func (*mockAuth) Name() string   { return "" }
 func (*mockAuth) String() string { return "" }
 
-func (s *SuiteRemote) TestConnectWithAuthWrongType(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, &mockAuth{}), Equals, ErrInvalidAuthMethod)
-	c.Assert(r.connected, Equals, false)
+func (s *RemoteSuite) TestConnectWithAuthWrongType(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.ConnectWithAuth(&mockAuth{}), Equals, ErrInvalidAuthMethod)
 }
 
-func (s *SuiteRemote) TestAlreadyConnected(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
+func (s *RemoteSuite) TestAlreadyConnected(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
+	defer func() {
+		c.Assert(r.Disconnect(), IsNil)
+	}()
 
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
-	defer func() { c.Assert(r.Disconnect(), IsNil) }()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), Equals, ErrAlreadyConnected)
-	c.Assert(r.connected, Equals, true)
+	c.Assert(r.Connect(), Equals, ErrAlreadyConnected)
 }
 
-func (s *SuiteRemote) TestDisconnect(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
+func (s *RemoteSuite) TestDisconnect(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 	c.Assert(r.Disconnect(), IsNil)
-	c.Assert(r.connected, Equals, false)
 }
 
-func (s *SuiteRemote) TestDisconnectedWhenNonConnected(c *C) {
-	r := NewGitUploadPackService()
+func (s *RemoteSuite) TestDisconnectedWhenNonConnected(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
 	c.Assert(r.Disconnect(), Equals, ErrNotConnected)
 }
 
-func (s *SuiteRemote) TestAlreadyDisconnected(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
+func (s *RemoteSuite) TestAlreadyDisconnected(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 	c.Assert(r.Disconnect(), IsNil)
 	c.Assert(r.Disconnect(), Equals, ErrNotConnected)
-	c.Assert(r.connected, Equals, false)
 }
 
-func (s *SuiteRemote) TestServeralConnections(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
+func (s *RemoteSuite) TestServeralConnections(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 	c.Assert(r.Disconnect(), IsNil)
 
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
-	c.Assert(r.connected, Equals, true)
+	c.Assert(r.Connect(), IsNil)
 	c.Assert(r.Disconnect(), IsNil)
-	c.Assert(r.connected, Equals, false)
 
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
-	c.Assert(r.connected, Equals, true)
+	c.Assert(r.Connect(), IsNil)
 	c.Assert(r.Disconnect(), IsNil)
-	c.Assert(r.connected, Equals, false)
 }
 
-func (s *SuiteRemote) TestInfoNotConnected(c *C) {
-	r := NewGitUploadPackService()
+func (s *RemoteSuite) TestInfoNotConnected(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
 	_, err := r.Info()
 	c.Assert(err, Equals, ErrNotConnected)
 }
 
-func (s *SuiteRemote) TestDefaultBranch(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
+func (s *RemoteSuite) TestDefaultBranch(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 	defer func() { c.Assert(r.Disconnect(), IsNil) }()
 
 	info, err := r.Info()
@@ -185,13 +93,9 @@ func (s *SuiteRemote) TestDefaultBranch(c *C) {
 	c.Assert(info.Capabilities.SymbolicReference("HEAD"), Equals, "refs/heads/master")
 }
 
-func (s *SuiteRemote) TestCapabilities(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
+func (s *RemoteSuite) TestCapabilities(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 	defer func() { c.Assert(r.Disconnect(), IsNil) }()
 
 	info, err := r.Info()
@@ -199,29 +103,26 @@ func (s *SuiteRemote) TestCapabilities(c *C) {
 	c.Assert(info.Capabilities.Get("agent").Values, HasLen, 1)
 }
 
-func (s *SuiteRemote) TestFetchNotConnected(c *C) {
-	r := NewGitUploadPackService()
+func (s *RemoteSuite) TestFetchNotConnected(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
 	pr := &common.GitUploadPackRequest{}
 	pr.Want(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
 	_, err := r.Fetch(pr)
 	c.Assert(err, Equals, ErrNotConnected)
 }
 
-func (s *SuiteRemote) TestFetch(c *C) {
-	agent, err := newSSHAgentConn()
-	c.Assert(err, IsNil)
-	defer func() { c.Assert(agent.close(), IsNil) }()
-
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(fixRepo, agent.auth), IsNil)
+func (s *RemoteSuite) TestFetch(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 	defer func() { c.Assert(r.Disconnect(), IsNil) }()
 
-	pr := &common.GitUploadPackRequest{}
-	pr.Want(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
-	reader, err := r.Fetch(pr)
+	req := &common.GitUploadPackRequest{}
+	req.Want(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
+	req.Want(core.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881"))
+	reader, err := r.Fetch(req)
 	c.Assert(err, IsNil)
 
 	b, err := ioutil.ReadAll(reader)
 	c.Assert(err, IsNil)
-	c.Assert(b, HasLen, 85374)
+	c.Assert(len(b), Equals, 85585)
 }
diff --git a/config/config.go b/config/config.go
index 4cf5f7c..7138952 100644
--- a/config/config.go
+++ b/config/config.go
@@ -3,8 +3,6 @@ package config
 import (
 	"errors"
 	"fmt"
-
-	"gopkg.in/src-d/go-git.v3/clients/common"
 )
 
 const (
@@ -40,10 +38,6 @@ func (c *RemoteConfig) Validate() error {
 		return ErrRemoteConfigEmptyURL
 	}
 
-	if _, err := common.NewEndpoint(c.URL); err != nil {
-		return err
-	}
-
 	if len(c.Fetch) == 0 {
 		c.Fetch = []RefSpec{RefSpec(fmt.Sprintf(DefaultRefSpec, c.Name))}
 	}
diff --git a/config/config_test.go b/config/config_test.go
index 853e518..d97053b 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -11,11 +11,6 @@ func (s *ConfigSuite) TestRemoteConfigValidateMissingURL(c *C) {
 	c.Assert(config.Validate(), Equals, ErrRemoteConfigEmptyURL)
 }
 
-func (s *ConfigSuite) TestRemoteConfigValidateInvalidURL(c *C) {
-	config := &RemoteConfig{Name: "foo", URL: "foo"}
-	c.Assert(config.Validate(), NotNil)
-}
-
 func (s *ConfigSuite) TestRemoteConfigValidateMissingName(c *C) {
 	config := &RemoteConfig{}
 	c.Assert(config.Validate(), Equals, ErrRemoteConfigEmptyName)
diff --git a/options.go b/options.go
index 0cca2e3..34f4dcd 100644
--- a/options.go
+++ b/options.go
@@ -3,7 +3,7 @@ package git
 import (
 	"errors"
 
-	"gopkg.in/src-d/go-git.v3/clients/common"
+	"gopkg.in/src-d/go-git.v4/clients/common"
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/core"
 )