Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test improvements; add server.CloseConnection method #39

Merged
merged 6 commits into from
Mar 18, 2021
18 changes: 17 additions & 1 deletion osc/osc.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type Server struct {
Addr string
Dispatcher Dispatcher
ReadTimeout time.Duration
close func() error
}

// Timetag represents an OSC Time Tag.
Expand Down Expand Up @@ -529,6 +530,8 @@ func (c *Client) Send(packet Packet) error {
// ListenAndServe retrieves incoming OSC packets and dispatches the retrieved
// OSC packets.
func (s *Server) ListenAndServe() error {
defer s.CloseConnection()

if s.Dispatcher == nil {
s.Dispatcher = NewStandardDispatcher()
}
Expand All @@ -537,7 +540,8 @@ func (s *Server) ListenAndServe() error {
if err != nil {
return err
}
defer ln.Close()

s.close = ln.Close

return s.Serve(ln)
}
Expand Down Expand Up @@ -568,6 +572,18 @@ func (s *Server) Serve(c net.PacketConn) error {
}
}

// CloseConnection forcibly closes a server's connection.
//
// This causes a "use of closed network connection" error the next time the
Copy link
Contributor

@glynternet glynternet Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling I found a way around this before but I can't remember it now 🤔
I think it involved using the read timeout value, but I wouldn't worry about working it out for the sake of the tests.

// server attempts to read from the connection.
func (s *Server) CloseConnection() error {
if s.close == nil {
return nil
}

return s.close()
}

// ReceivePacket listens for incoming OSC packets and returns the packet if one is received.
func (s *Server) ReceivePacket(c net.PacketConn) (Packet, error) {
return s.readFromConnection(c)
Expand Down
84 changes: 62 additions & 22 deletions osc/osc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"bytes"
"net"
"reflect"
"strconv"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -111,53 +113,83 @@ func TestAddMsgHandlerWithInvalidAddress(t *testing.T) {
}
}

// These tests stop the server by forcibly closing the connection, which causes
// a "use of closed network connection" error the next time we try to read from
// the connection. As a workaround, this wraps server.ListenAndServe() in an
// error-handling layer that doesn't consider "use of closed network connection"
// an error.
//
// Open question: is this desired behavior, or should server.serve return
// successfully in cases where it would otherwise throw this error?
func serveUntilInterrupted(server *Server) error {
if err := server.ListenAndServe(); err != nil &&
!strings.Contains(err.Error(), "use of closed network connection") {
return err
}

return nil
}

func TestServerMessageDispatching(t *testing.T) {
finish := make(chan bool)
start := make(chan bool)
done := sync.WaitGroup{}
done.Add(2)

// Start the OSC server in a new go-routine
go func() {
conn, err := net.ListenPacket("udp", "localhost:6677")
if err != nil {
t.Fatal(err)
port := 6677
addr := "localhost:" + strconv.Itoa(port)

d := NewStandardDispatcher()
server := &Server{Addr: addr, Dispatcher: d}

defer func() {
if err := server.CloseConnection(); err != nil {
t.Error(err)
}
defer conn.Close()
}()

d := NewStandardDispatcher()
daveyarwood marked this conversation as resolved.
Show resolved Hide resolved
err = d.AddMsgHandler("/address/test", func(msg *Message) {
if err := d.AddMsgHandler(
"/address/test",
func(msg *Message) {
if len(msg.Arguments) != 1 {
t.Error("Argument length should be 1 and is: " + string(len(msg.Arguments)))
t.Errorf("Argument length should be 1 and is: %d", len(msg.Arguments))
}

if msg.Arguments[0].(int32) != 1122 {
t.Error("Argument should be 1122 and is: " + string(msg.Arguments[0].(int32)))
}

// Stop OSC server
conn.Close()
finish <- true
})
if err != nil {
t.Error("Error adding message handler")
}
},
); err != nil {
t.Error("Error adding message handler")
}

server := &Server{Addr: "localhost:6677", Dispatcher: d}
// Server goroutine
go func() {
start <- true
server.Serve(conn)

if err := serveUntilInterrupted(server); err != nil {
t.Errorf("error during Serve: %s", err.Error())
}
}()

// Client goroutine
go func() {
timeout := time.After(5 * time.Second)
select {
case <-timeout:
case <-start:
time.Sleep(500 * time.Millisecond)
client := NewClient("localhost", 6677)
client := NewClient("localhost", port)
msg := NewMessage("/address/test")
msg.Append(int32(1122))
client.Send(msg)
if err := client.Send(msg); err != nil {
t.Error(err)
done.Done()
hypebeast marked this conversation as resolved.
Show resolved Hide resolved
done.Done()
return
}
}

done.Done()
Expand All @@ -173,6 +205,8 @@ func TestServerMessageDispatching(t *testing.T) {
}

func TestServerMessageReceiving(t *testing.T) {
port := 6677

finish := make(chan bool)
start := make(chan bool)
done := sync.WaitGroup{}
Expand All @@ -181,7 +215,8 @@ func TestServerMessageReceiving(t *testing.T) {
// Start the server in a go-routine
go func() {
server := &Server{}
c, err := net.ListenPacket("udp", "localhost:6677")

c, err := net.ListenPacket("udp", "localhost:"+strconv.Itoa(port))
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -220,12 +255,17 @@ func TestServerMessageReceiving(t *testing.T) {
select {
case <-timeout:
case <-start:
client := NewClient("localhost", 6677)
client := NewClient("localhost", port)
msg := NewMessage("/address/test")
msg.Append(int32(1122))
msg.Append(int32(3344))
time.Sleep(500 * time.Millisecond)
client.Send(msg)
if err := client.Send(msg); err != nil {
t.Error(err)
done.Done()
done.Done()
return
}
}

done.Done()
Expand Down