Repository: ably/ably-go (go)

Issue:
Use the net/http/httptest package to write unit test(s) for websockets
The code in `websockets.go` is currently not covered by any unit tests. We have an open issue for replacing the the websocket package used by the project https://github.com/ably/ably-go/issues/368 a PR which was raised to attempt this work https://github.com/ably/ably-go/pull/433 appears to introduce new intermittent failure to the integration tests for this project. As we have customers which rely on this SDK, we need a high degree of certainty that switching the existing websocket package everything continues to work as before and no new issues have been introduced. 

To ensure stability after switching the websocket package we should implement some unit tests around the websocket connection **before** switching the package. This would give us a higher level of confidence that changing the package continues to support both json and binary formats and has not introduced any new issues.

Acceptance criteria:
* Write unit test(s) that create a new [test server](https://godoc.org/net/http/httptest#Server) using the [net/http/httptest](https://godoc.org/net/http/httptest) package. 
* As this test server does not need to use the same websocket package as the client and we are trying to migrate away from the [golang.org/x/net/websocket](https://pkg.go.dev/golang.org/x/net/websocket) package, the test server should use the [nhooyr.io/websocket](https://pkg.go.dev/nhooyr.io/websocket) package. 
* The test server should allow the code under test to read and write messages to test the connection.
* The test server should allow the code under test to send and receive both json and binary message formats. 
* Any new tests added must be stable as they will run in the CI pipeline

┆Issue is synchronized with this [Jira Uncategorised](https://ably.atlassian.net/browse/SDK-1966) by [Unito](https://www.unito.io)
┆sprint: SDK 2022-Q2-S7  23/06


Pull request:
Add unit tests for websocket.go
This PR increases unit test coverage for `websocket.go` from 0% to 81.2%

Resolves #541 

Patch:
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
index 4ead3ad..05bb839 100644
--- a/.github/workflows/check.yml
+++ b/.github/workflows/check.yml
@@ -48,7 +48,7 @@ jobs:
 
       - name: Upload test results
         if: always()
-        uses: ably/test-observability-action@v1
+        uses: ably-labs/test-observability-action@main
         with:
           server-url: 'https://test-observability.herokuapp.com'
           server-auth: ${{ secrets.TEST_OBSERVABILITY_SERVER_AUTH_KEY }}
diff --git a/go.mod b/go.mod
index d6e0e27..ce6ec28 100644
--- a/go.mod
+++ b/go.mod
@@ -5,10 +5,12 @@ require (
 	github.com/ugorji/go/codec v1.1.9
 	golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f
 	golang.org/x/sys v0.0.0-20211019181941-9d821ace8654
+	nhooyr.io/websocket v1.8.7
 )
 
 require (
-	github.com/davecgh/go-spew v1.1.0 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/klauspost/compress v1.10.3 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
 )
diff --git a/go.sum b/go.sum
index 7b12e85..e455bcc 100644
--- a/go.sum
+++ b/go.sum
@@ -1,24 +1,76 @@
-github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE=
+github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI=
+github.com/gin-gonic/gin v1.6.3 h1:ahKqKTFpO5KTPHxWZjEdPScmYaGtLo8Y4DMHoEsnp14=
+github.com/gin-gonic/gin v1.6.3/go.mod h1:75u5sXoLsGZoRN5Sgbi1eraJ4GU3++wFwWzhwvtwp4M=
+github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4=
+github.com/go-playground/locales v0.13.0 h1:HyWk6mgj5qFqCT5fjGBuRArbVDfE4hi8+e8ceBS/t7Q=
+github.com/go-playground/locales v0.13.0/go.mod h1:taPMhCMXrRLJO55olJkUXHZBHCxTMfnGwq/HNwmWNS8=
+github.com/go-playground/universal-translator v0.17.0 h1:icxd5fm+REJzpZx7ZfpaD876Lmtgy7VtROAbHHXk8no=
+github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA=
+github.com/go-playground/validator/v10 v10.2.0 h1:KgJ0snyC2R9VXYN2rneOtQcw5aHQB1Vv0sFl1UcHBOY=
+github.com/go-playground/validator/v10 v10.2.0/go.mod h1:uOYAAleCW8F/7oMFd6aG0GOhaH6EGOAJShg8Id5JGkI=
+github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee h1:s+21KNqlpePfkah2I+gwHF8xmJWRjooY+5248k6m4A0=
+github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee/go.mod h1:L0fX3K22YWvt/FAX9NnzrNzcI4wNYi9Yku4O0LKYflo=
+github.com/gobwas/pool v0.2.0 h1:QEmUOlnSjWtnpRGHF3SauEiOsy82Cup83Vf2LcMlnc8=
+github.com/gobwas/pool v0.2.0/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
+github.com/gobwas/ws v1.0.2 h1:CoAavW/wd/kulfZmSIBt6p24n4j7tHgNVCjsfHVNUbo=
+github.com/gobwas/ws v1.0.2/go.mod h1:szmBTxLgaFppYjEmNtny/v3w89xOydFnnZMcgRRu/EM=
+github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw=
+github.com/golang/protobuf v1.3.5 h1:F768QJ1E9tib+q5Sc8MkdJi1RxLTbRcTf8LJV56aRls=
+github.com/golang/protobuf v1.3.5/go.mod h1:6O5/vntMXwX2lRkT1hjjk0nAC1IDOTvTlVgjlRvqsdk=
+github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
+github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
+github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvKCM=
+github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
+github.com/json-iterator/go v1.1.9 h1:9yzud/Ht36ygwatGx56VwCZtlI/2AD15T1X2sjSuGns=
+github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
+github.com/klauspost/compress v1.10.3 h1:OP96hzwJVBIHYU52pVTI6CczrxPvrGfgqF9N5eTO0Q8=
+github.com/klauspost/compress v1.10.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
+github.com/leodido/go-urn v1.2.0 h1:hpXL4XnriNwQ/ABnpepYM/1vCLWNDfUNts8dX3xTG6Y=
+github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII=
+github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY=
+github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
+github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421 h1:ZqeYNhU3OHLH3mGKHDcjJRFFRrJa6eAM5H+CtDdOsPc=
+github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
+github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 h1:Esafd1046DLDQ0W1YjYsBW+p8U2u7vzgW2SQVmlNazg=
+github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw=
 github.com/ugorji/go v1.1.9 h1:SObrQTaSuP8WOv2WNCj8gECiNSJIUvk3Q7N26c96Gws=
 github.com/ugorji/go v1.1.9/go.mod h1:chLrngdsg43geAaeId+nXO57YsDdl5OZqd/QtBiD19g=
+github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY=
 github.com/ugorji/go/codec v1.1.9 h1:J/7hhpkQwgypRNvaeh/T5gzJ2gEI/l8S3qyRrdEa1fA=
 github.com/ugorji/go/codec v1.1.9/go.mod h1:+SWgpdqOgdW5sBaiDfkHilQ1SxQ1hBkq/R+kHfL7Suo=
 golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f h1:OfiFi4JbukWwe3lzw+xunroH1mnC1e2Gy5cxNJApiSY=
 golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
+golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
 golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
+gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+nhooyr.io/websocket v1.8.7 h1:usjR2uOr/zjjkVMy0lW+PPohFok7PCow5sDjLgX4P4g=
+nhooyr.io/websocket v1.8.7/go.mod h1:B70DZP8IakI65RVQ51MsWP/8jndNma26DVA/nFSCgW0=


Test patch:
diff --git a/ably/websocket_internal_test.go b/ably/websocket_internal_test.go
new file mode 100644
index 0000000..d5aecc1
--- /dev/null
+++ b/ably/websocket_internal_test.go
@@ -0,0 +1,192 @@
+//go:build !integration
+// +build !integration
+
+package ably
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"log"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/ugorji/go/codec"
+	"nhooyr.io/websocket"
+)
+
+var timeout = time.Second * 30
+
+// handleWebsocketHandshake is a handler that can handle a websocket handshake and connection from a client.
+func handleWebsocketHandshake(w http.ResponseWriter, r *http.Request) {
+	conn, err := websocket.Accept(w, r, &websocket.AcceptOptions{})
+	if err != nil {
+		log.Printf("test server: error accepting websocket handshake: %+v\n", err)
+	}
+	defer conn.Close(websocket.StatusNormalClosure, "")
+}
+
+// handleWebsocketMessages is a handler that receives a protocol message over a websocket connection then
+// responds by sending a protocol message back to the client including the message type and original message.
+func handleWebsocketMessage(w http.ResponseWriter, r *http.Request) {
+	conn, err := websocket.Accept(w, r, &websocket.AcceptOptions{})
+	if err != nil {
+		log.Printf("test server: error accepting websocket handshake: %+v\n", err)
+	}
+	defer conn.Close(websocket.StatusInternalError, "connection closed")
+
+	ctx, cancel := context.WithTimeout(r.Context(), time.Second*10)
+	defer cancel()
+
+	// Read message received from client.
+	msgType, received, err := conn.Read(ctx)
+	if err != nil {
+		log.Printf("test server: error reading received message: %+v\n", err)
+	}
+
+	// Reply to message with the type of message received and the message string.
+	reply := protocolMessage{
+		Action:   actionMessage,
+		Messages: []*Message{{Name: msgType.String(), Data: string(received)}},
+	}
+
+	var payloadType websocket.MessageType
+	var payload []byte
+
+	switch msgType.String() {
+	case "MessageText":
+		payloadType = websocket.MessageText
+
+		payload, err = json.Marshal(reply)
+		if err != nil {
+			log.Printf("test server: error marshalling json: %+v\n", err)
+		}
+
+	case "MessageBinary":
+		payloadType = websocket.MessageBinary
+
+		var handle codec.MsgpackHandle
+		var buf bytes.Buffer
+		enc := codec.NewEncoder(&buf, &handle)
+		if err := enc.Encode(reply); err != nil {
+			log.Printf("test server: error encoding msg: %+v\n", err)
+		}
+		payload = buf.Bytes()
+	}
+
+	if err := conn.Write(ctx, payloadType, payload); err != nil {
+		log.Println("test server: error sending message\n", err)
+	}
+}
+
+func TestWebsocketDial(t *testing.T) {
+	// Create test server with a handler that will accept a websocket handshake.
+	ts := httptest.NewServer(http.HandlerFunc(handleWebsocketHandshake))
+	defer ts.Close()
+
+	// Convert http:// to ws:// and parse the test server url
+	websocketUrl := fmt.Sprintf("ws%s", strings.TrimPrefix(ts.URL, "http"))
+	testServerURL, _ := url.Parse(websocketUrl)
+
+	// Calculate the expected Ably-Agent header used by tests.
+	expectedAgentHeader := AblySDKIdentifier + " " + GoRuntimeIdentifier + " " + GoOSIdentifier()
+
+	tests := map[string]struct {
+		dialProtocol        string
+		expectedPayloadType byte
+		expectedSubprotocol []string
+		expectedResult      *websocketConn
+		expectedErr         error
+	}{
+		"Can dial for protocol application/json": {
+			dialProtocol:        "application/json",
+			expectedPayloadType: uint8(1),
+			expectedSubprotocol: []string(nil),
+			expectedErr:         nil,
+		},
+		"Can dial for protocol application/x-msgpack": {
+			dialProtocol:        "application/x-msgpack",
+			expectedPayloadType: uint8(1),
+			expectedSubprotocol: []string(nil),
+			expectedErr:         nil,
+		},
+		"Can handle an error when dialing for an invalid protocol": {
+			dialProtocol:   "aProtocol",
+			expectedResult: nil,
+			expectedErr:    errors.New(`invalid protocol "aProtocol"`),
+		},
+	}
+
+	for testName, test := range tests {
+		t.Run(testName, func(t *testing.T) {
+
+			result, err := dialWebsocket(test.dialProtocol, testServerURL, timeout)
+			assert.Equal(t, test.expectedErr, err)
+
+			if result != nil {
+				assert.NotNil(t, result.codec)
+				assert.Equal(t, expectedAgentHeader, result.conn.Config().Header.Get("Ably-Agent"))
+				assert.Equal(t, test.expectedPayloadType, result.conn.PayloadType)
+				assert.Equal(t, test.expectedSubprotocol, result.conn.Config().Protocol)
+				assert.True(t, result.conn.IsClientConn())
+			}
+		})
+	}
+}
+
+func TestWebsocketSendAndReceive(t *testing.T) {
+	// Create test server with a handler that can receive a message.
+	ts := httptest.NewServer(http.HandlerFunc(handleWebsocketMessage))
+	defer ts.Close()
+
+	// Convert http:// to ws:// and parse the test server url
+	websocketUrl := fmt.Sprintf("ws%s", strings.TrimPrefix(ts.URL, "http"))
+	testServerURL, _ := url.Parse(websocketUrl)
+
+	tests := map[string]struct {
+		dialProtocol        string
+		expectedMessageType string
+		expectedPayloadType byte
+		expectedResult      *websocketConn
+		expectedErr         error
+	}{
+		"Can send and receive a message using protocol application/json": {
+			dialProtocol:        "application/json",
+			expectedMessageType: "MessageText",
+			expectedErr:         nil,
+		},
+		"Can send and receive a message using protocol application/x-msgpack": {
+			dialProtocol:        "application/x-msgpack",
+			expectedMessageType: "MessageBinary",
+			expectedErr:         nil,
+		},
+	}
+
+	for testName, test := range tests {
+		t.Run(testName, func(t *testing.T) {
+
+			ws, err := dialWebsocket(test.dialProtocol, testServerURL, timeout)
+			assert.NoError(t, err)
+			msg := protocolMessage{
+				Messages: []*Message{{Name: "temperature", Data: "22.7"}},
+			}
+
+			ws.Send(&msg)
+			result, err := ws.Receive(time.Now().Add(timeout))
+
+			assert.Equal(t, test.expectedErr, err)
+			assert.Equal(t, test.expectedMessageType, result.Messages[0].Name)
+			assert.Equal(t, 1, len(result.Messages))
+			assert.Equal(t, actionMessage, result.Action)
+			assert.Contains(t, result.Messages[0].Data, "temperature")
+			assert.Contains(t, result.Messages[0].Data, "22.7")
+		})
+	}
+}