From 2ad899b06696d118631c19bb2393997eaea071b2 Mon Sep 17 00:00:00 2001 From: Mikhail Bragin Date: Fri, 21 Jan 2022 13:52:19 +0100 Subject: [PATCH] Test conn (#199) * test: add conn tests * test: add ConnStatus tests * test: add error test * test: add more conn tests --- client/internal/engine.go | 10 +- client/internal/peer/conn.go | 19 ++-- client/internal/peer/conn_test.go | 144 ++++++++++++++++++++++++++++ client/internal/peer/error.go | 16 ++++ client/internal/peer/error_test.go | 27 ++++++ client/internal/peer/status_test.go | 27 ++++++ go.mod | 9 +- go.sum | 2 + 8 files changed, 245 insertions(+), 9 deletions(-) create mode 100644 client/internal/peer/conn_test.go create mode 100644 client/internal/peer/error_test.go create mode 100644 client/internal/peer/status_test.go diff --git a/client/internal/engine.go b/client/internal/engine.go index 18c340eef..8b4e3dfec 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -193,7 +193,15 @@ func (e *Engine) removePeer(peerKey string) error { conn, exists := e.peerConns[peerKey] if exists { delete(e.peerConns, peerKey) - return conn.Close() + err := conn.Close() + if err != nil { + switch err.(type) { + case *peer.ConnectionAlreadyClosedError: + return nil + default: + return err + } + } } return nil } diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index 2458284ca..f9324af3e 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -350,6 +350,7 @@ func (conn *Conn) Close() error { defer conn.mu.Unlock() select { case conn.closeCh <- struct{}{}: + return nil default: // probably could happen when peer has been added and removed right after not even starting to connect // todo further investigate @@ -364,8 +365,8 @@ func (conn *Conn) Close() error { // engine adds a new Conn for 4 and 5 // therefore peer 4 has 2 Conn objects log.Warnf("closing not started coonection %s", conn.config.Key) + return NewConnectionAlreadyClosed(conn.config.Key) } - return nil } // Status returns current status of the Conn @@ -375,29 +376,33 @@ func (conn *Conn) Status() ConnStatus { return conn.status } -// OnRemoteOffer handles an offer from the remote peer -// can block until Conn restarts -func (conn *Conn) OnRemoteOffer(remoteAuth IceCredentials) { +// OnRemoteOffer handles an offer from the remote peer and returns true if the message was accepted, false otherwise +// doesn't block, discards the message if connection wasn't ready +func (conn *Conn) OnRemoteOffer(remoteAuth IceCredentials) bool { log.Debugf("OnRemoteOffer from peer %s on status %s", conn.config.Key, conn.status.String()) select { case conn.remoteOffersCh <- remoteAuth: + return true default: log.Debugf("OnRemoteOffer skipping message from peer %s on status %s because is not ready", conn.config.Key, conn.status.String()) //connection might not be ready yet to receive so we ignore the message + return false } } -// OnRemoteAnswer handles an offer from the remote peer -// can block until Conn restarts -func (conn *Conn) OnRemoteAnswer(remoteAuth IceCredentials) { +// OnRemoteAnswer handles an offer from the remote peer and returns true if the message was accepted, false otherwise +// doesn't block, discards the message if connection wasn't ready +func (conn *Conn) OnRemoteAnswer(remoteAuth IceCredentials) bool { log.Debugf("OnRemoteAnswer from peer %s on status %s", conn.config.Key, conn.status.String()) select { case conn.remoteAnswerCh <- remoteAuth: + return true default: //connection might not be ready yet to receive so we ignore the message log.Debugf("OnRemoteAnswer skipping message from peer %s on status %s because is not ready", conn.config.Key, conn.status.String()) + return false } } diff --git a/client/internal/peer/conn_test.go b/client/internal/peer/conn_test.go new file mode 100644 index 000000000..fcdf1840c --- /dev/null +++ b/client/internal/peer/conn_test.go @@ -0,0 +1,144 @@ +package peer + +import ( + "github.com/magiconair/properties/assert" + "github.com/pion/ice/v2" + "github.com/wiretrustee/wiretrustee/client/internal/proxy" + "sync" + "testing" + "time" +) + +var connConf = ConnConfig{ + Key: "LLHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=", + LocalKey: "RRHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=", + StunTurn: []*ice.URL{}, + InterfaceBlackList: nil, + Timeout: time.Second, + ProxyConfig: proxy.Config{}, +} + +func TestConn_GetKey(t *testing.T) { + conn, err := NewConn(connConf) + if err != nil { + return + } + + got := conn.GetKey() + + assert.Equal(t, got, connConf.Key, "they should be equal") +} + +func TestConn_OnRemoteOffer(t *testing.T) { + + conn, err := NewConn(connConf) + if err != nil { + return + } + + wg := sync.WaitGroup{} + wg.Add(2) + go func() { + <-conn.remoteOffersCh + wg.Done() + }() + + go func() { + for { + accepted := conn.OnRemoteOffer(IceCredentials{ + UFrag: "test", + Pwd: "test", + }) + if accepted { + wg.Done() + return + } + } + }() + + wg.Wait() +} + +func TestConn_OnRemoteAnswer(t *testing.T) { + + conn, err := NewConn(connConf) + if err != nil { + return + } + + wg := sync.WaitGroup{} + wg.Add(2) + go func() { + <-conn.remoteAnswerCh + wg.Done() + }() + + go func() { + for { + accepted := conn.OnRemoteAnswer(IceCredentials{ + UFrag: "test", + Pwd: "test", + }) + if accepted { + wg.Done() + return + } + } + }() + + wg.Wait() +} +func TestConn_Status(t *testing.T) { + + conn, err := NewConn(connConf) + if err != nil { + return + } + + tables := []struct { + name string + status ConnStatus + want ConnStatus + }{ + {"StatusConnected", StatusConnected, StatusConnected}, + {"StatusDisconnected", StatusDisconnected, StatusDisconnected}, + {"StatusConnecting", StatusConnecting, StatusConnecting}, + } + + for _, table := range tables { + t.Run(table.name, func(t *testing.T) { + conn.status = table.status + + got := conn.Status() + assert.Equal(t, got, table.want, "they should be equal") + }) + } +} + +func TestConn_Close(t *testing.T) { + + conn, err := NewConn(connConf) + if err != nil { + return + } + + wg := sync.WaitGroup{} + wg.Add(1) + go func() { + <-conn.closeCh + wg.Done() + }() + + go func() { + for { + err := conn.Close() + if err != nil { + continue + } else { + return + } + } + }() + + wg.Wait() +} diff --git a/client/internal/peer/error.go b/client/internal/peer/error.go index 91d69532b..3c27d8afb 100644 --- a/client/internal/peer/error.go +++ b/client/internal/peer/error.go @@ -54,3 +54,19 @@ func NewConnectionDisconnectedError(peer string) error { peer: peer, } } + +// ConnectionAlreadyClosedError is an error indicating that a peer Conn has been already closed and the invocation of the Close() method has been performed over a closed connection +type ConnectionAlreadyClosedError struct { + peer string +} + +func (e *ConnectionAlreadyClosedError) Error() string { + return fmt.Sprintf("connection to peer %s has been already closed", e.peer) +} + +// NewConnectionAlreadyClosed creates a new ConnectionAlreadyClosedError error +func NewConnectionAlreadyClosed(peer string) error { + return &ConnectionAlreadyClosedError{ + peer: peer, + } +} diff --git a/client/internal/peer/error_test.go b/client/internal/peer/error_test.go new file mode 100644 index 000000000..7757537ee --- /dev/null +++ b/client/internal/peer/error_test.go @@ -0,0 +1,27 @@ +package peer + +import ( + "github.com/stretchr/testify/assert" + "testing" + "time" +) + +func TestNewConnectionClosedError(t *testing.T) { + err := NewConnectionClosedError("X") + assert.Equal(t, &ConnectionClosedError{peer: "X"}, err) +} + +func TestNewConnectionDisconnectedError(t *testing.T) { + err := NewConnectionDisconnectedError("X") + assert.Equal(t, &ConnectionDisconnectedError{peer: "X"}, err) +} + +func TestNewConnectionTimeoutErrorC(t *testing.T) { + err := NewConnectionTimeoutError("X", time.Second) + assert.Equal(t, &ConnectionTimeoutError{peer: "X", timeout: time.Second}, err) +} + +func TestNewConnectionAlreadyClosed(t *testing.T) { + err := NewConnectionAlreadyClosed("X") + assert.Equal(t, &ConnectionAlreadyClosedError{peer: "X"}, err) +} diff --git a/client/internal/peer/status_test.go b/client/internal/peer/status_test.go new file mode 100644 index 000000000..54b172d3f --- /dev/null +++ b/client/internal/peer/status_test.go @@ -0,0 +1,27 @@ +package peer + +import ( + "github.com/magiconair/properties/assert" + "testing" +) + +func TestConnStatus_String(t *testing.T) { + + tables := []struct { + name string + status ConnStatus + want string + }{ + {"StatusConnected", StatusConnected, "StatusConnected"}, + {"StatusDisconnected", StatusDisconnected, "StatusDisconnected"}, + {"StatusConnecting", StatusConnecting, "StatusConnecting"}, + } + + for _, table := range tables { + t.Run(table.name, func(t *testing.T) { + got := table.status.String() + assert.Equal(t, got, table.want, "they should be equal") + }) + } + +} diff --git a/go.mod b/go.mod index bb7a3b98d..393b7dc58 100644 --- a/go.mod +++ b/go.mod @@ -27,10 +27,15 @@ require ( gopkg.in/natefinch/lumberjack.v2 v2.0.0 ) -require github.com/rs/xid v1.3.0 +require ( + github.com/magiconair/properties v1.8.5 + github.com/rs/xid v1.3.0 + github.com/stretchr/testify v1.7.0 +) require ( github.com/BurntSushi/toml v0.4.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/fsnotify/fsnotify v1.5.1 // indirect github.com/google/go-cmp v0.5.6 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect @@ -47,6 +52,7 @@ require ( github.com/pion/transport v0.12.3 // indirect github.com/pion/turn/v2 v2.0.5 // indirect github.com/pion/udp v0.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df // indirect golang.org/x/mod v0.5.1 // indirect golang.org/x/net v0.0.0-20211215060638-4ddde0e984e9 // indirect @@ -58,5 +64,6 @@ require ( google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa // indirect gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect honnef.co/go/tools v0.2.2 // indirect ) diff --git a/go.sum b/go.sum index 3a3a77fab..28c60dc00 100644 --- a/go.sum +++ b/go.sum @@ -148,6 +148,7 @@ github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4= github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -287,6 +288,7 @@ github.com/leodido/go-urn v1.1.0/go.mod h1:+cyI34gQWZcE1eQU7NVgKkkzdXDQHr1dBMtdA github.com/lxn/walk v0.0.0-20210112085537-c389da54e794/go.mod h1:E23UucZGqpuUANJooIbHWCufXvOcT6E7Stq81gU+CSQ= github.com/lxn/win v0.0.0-20210218163916-a377121e959e/go.mod h1:KxxjdtRkfNoYDCUP5ryK7XJJNTnpC8atvtmTheChOtk= github.com/lyft/protoc-gen-star v0.5.3/go.mod h1:V0xaHgaf5oCCqmcxYcWiDfTiKsZsRc87/1qhoTACD8w= +github.com/magiconair/properties v1.8.5 h1:b6kJs+EmPFMYGkow9GiUyCyOvIwYetYJ3fSaWak/Gls= github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=