Merge pull request #123 from nhooyr/test

Improve tests

Author: nhooyr

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2
 jobs:
   fmt:
     docker:
-      - image: nhooyr/websocket-ci
+      - image: nhooyr/websocket-ci@sha256:371ca985ce2548840aeb0f8434a551708cdfe0628be722c361958e65cdded945
     steps:
       - checkout
       - restore_cache:
@@ -19,7 +19,7 @@ jobs:
 
   lint:
     docker:
-      - image: nhooyr/websocket-ci
+      - image: nhooyr/websocket-ci@sha256:371ca985ce2548840aeb0f8434a551708cdfe0628be722c361958e65cdded945
     steps:
       - checkout
       - restore_cache:
@@ -36,7 +36,7 @@ jobs:
 
   test:
     docker:
-      - image: nhooyr/websocket-ci
+      - image: nhooyr/websocket-ci@sha256:371ca985ce2548840aeb0f8434a551708cdfe0628be722c361958e65cdded945
     steps:
       - checkout
       - restore_cache:

README.md

@@ -34,7 +34,7 @@ For a production quality example that shows off the full API, see the [echo exam
 
 ```go
 http.HandlerFunc(func (w http.ResponseWriter, r *http.Request) {
-	c, err := websocket.Accept(w, r, websocket.AcceptOptions{})
+	c, err := websocket.Accept(w, r, nil)
 	if err != nil {
 		// ...
 	}
@@ -64,7 +64,7 @@ in net/http](https://github.com/golang/go/issues/26937#issuecomment-415855861) t
 ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 defer cancel()
 
-c, _, err := websocket.Dial(ctx, "ws://localhost:8080", websocket.DialOptions{})
+c, _, err := websocket.Dial(ctx, "ws://localhost:8080", nil)
 if err != nil {
 	// ...
 }

accept.go

@@ -84,15 +84,19 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) error {
 //
 // If an error occurs, Accept will always write an appropriate response so you do not
 // have to.
-func Accept(w http.ResponseWriter, r *http.Request, opts AcceptOptions) (*Conn, error) {
+func Accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (*Conn, error) {
 	c, err := accept(w, r, opts)
 	if err != nil {
 		return nil, xerrors.Errorf("failed to accept websocket connection: %w", err)
 	}
 	return c, nil
 }
 
-func accept(w http.ResponseWriter, r *http.Request, opts AcceptOptions) (*Conn, error) {
+func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (*Conn, error) {
+	if opts == nil {
+		opts = &AcceptOptions{}
+	}
+
 	err := verifyClientRequest(w, r)
 	if err != nil {
 		return nil, err

accept_test.go

@@ -6,6 +6,39 @@ import (
 	"testing"
 )
 
+func TestAccept(t *testing.T) {
+	t.Parallel()
+
+	t.Run("badClientHandshake", func(t *testing.T) {
+		t.Parallel()
+
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest("GET", "/", nil)
+
+		_, err := Accept(w, r, nil)
+		if err == nil {
+			t.Fatalf("unexpected error value: %v", err)
+		}
+
+	})
+
+	t.Run("requireHttpHijacker", func(t *testing.T) {
+		t.Parallel()
+
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest("GET", "/", nil)
+		r.Header.Set("Connection", "Upgrade")
+		r.Header.Set("Upgrade", "websocket")
+		r.Header.Set("Sec-WebSocket-Version", "13")
+		r.Header.Set("Sec-WebSocket-Key", "meow123")
+
+		_, err := Accept(w, r, nil)
+		if err == nil || !strings.Contains(err.Error(), "http.Hijacker") {
+			t.Fatalf("unexpected error value: %v", err)
+		}
+	})
+}
+
 func Test_verifyClientHandshake(t *testing.T) {
 	t.Parallel()
 

ci/image/Dockerfile

@@ -6,8 +6,7 @@ ENV GOFLAGS="-mod=readonly"
 ENV PAGER=cat
 
 RUN apt-get update && \
-  apt-get install -y shellcheck python-pip npm && \
-  pip2 install autobahntestsuite && \
+  apt-get install -y shellcheck npm && \
   npm install -g prettier
 
-RUN git config --global color.ui always
+RUN git config --global color.ui always

ci/test.sh

@@ -4,19 +4,34 @@ set -euo pipefail
 cd "$(dirname "${0}")"
 cd "$(git rev-parse --show-toplevel)"
 
-mkdir -p ci/out/websocket
-testFlags=(
+argv=(
+  go run gotest.tools/gotestsum
+  # https://circleci.com/docs/2.0/collect-test-data/
+  "--junitfile=ci/out/websocket/testReport.xml"
+  "--format=short-verbose"
+  --
   -race
   "-vet=off"
   "-bench=."
+)
+# Interactive usage probably does not want to enable benchmarks, race detection
+# turn off vet or use gotestsum by default.
+if [[ $# -gt 0 ]]; then
+  argv=(go test "$@")
+fi
+
+# We always want coverage.
+argv+=(
   "-coverprofile=ci/out/coverage.prof"
   "-coverpkg=./..."
 )
-# https://circleci.com/docs/2.0/collect-test-data/
-go run gotest.tools/gotestsum \
-  --junitfile ci/out/websocket/testReport.xml \
-  --format=short-verbose \
-  -- "${testFlags[@]}"
+
+mkdir -p ci/out/websocket
+"${argv[@]}"
+
+# Removes coverage of generated files.
+grep -v _string.go < ci/out/coverage.prof > ci/out/coverage2.prof
+mv ci/out/coverage2.prof ci/out/coverage.prof
 
 go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html
 if [[ ${CI:-} ]]; then

dial.go

@@ -41,15 +41,23 @@ type DialOptions struct {
 // This function requires at least Go 1.12 to succeed as it uses a new feature
 // in net/http to perform WebSocket handshakes and get a writable body
 // from the transport. See https://github.com/golang/go/issues/26937#issuecomment-415855861
-func Dial(ctx context.Context, u string, opts DialOptions) (*Conn, *http.Response, error) {
+func Dial(ctx context.Context, u string, opts *DialOptions) (*Conn, *http.Response, error) {
 	c, r, err := dial(ctx, u, opts)
 	if err != nil {
 		return nil, r, xerrors.Errorf("failed to websocket dial: %w", err)
 	}
 	return c, r, nil
 }
 
-func dial(ctx context.Context, u string, opts DialOptions) (_ *Conn, _ *http.Response, err error) {
+func dial(ctx context.Context, u string, opts *DialOptions) (_ *Conn, _ *http.Response, err error) {
+	if opts == nil {
+		opts = &DialOptions{}
+	}
+
+	// Shallow copy to ensure defaults do not affect user passed options.
+	opts2 := *opts
+	opts = &opts2
+
 	if opts.HTTPClient == nil {
 		opts.HTTPClient = http.DefaultClient
 	}

dial_test.go

@@ -1,11 +1,60 @@
 package websocket
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 )
 
+func TestBadDials(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name string
+		url  string
+		opts *DialOptions
+	}{
+		{
+			name: "badURL",
+			url:  "://noscheme",
+		},
+		{
+			name: "badURLScheme",
+			url:  "ftp://nhooyr.io",
+		},
+		{
+			name: "badHTTPClient",
+			url:  "ws://nhooyr.io",
+			opts: &DialOptions{
+				HTTPClient: &http.Client{
+					Timeout: time.Minute,
+				},
+			},
+		},
+		{
+			name: "badTLS",
+			url:  "wss://totallyfake.nhooyr.io",
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+			defer cancel()
+
+			_, _, err := Dial(ctx, tc.url, tc.opts)
+			if err == nil {
+				t.Fatalf("expected non nil error: %+v", err)
+			}
+		})
+	}
+}
+
 func Test_verifyServerHandshake(t *testing.T) {
 	t.Parallel()
 

docs/CONTRIBUTING.md

@@ -34,16 +34,21 @@ browse coverage.
 
 You can run CI locally. The various steps are located in `ci/*.sh`.
 
-1. `ci/fmt.sh` requires node (specifically prettier).
-1. `ci/lint.sh` requires [shellcheck](https://github.com/koalaman/shellcheck#installing).
-1. `ci/test.sh` requires the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite).
-1. `ci/run.sh` runs the above scripts in order.
+1. `ci/fmt.sh` which requires node (specifically prettier).
+1. `ci/lint.sh` which requires [shellcheck](https://github.com/koalaman/shellcheck#installing).
+1. `ci/test.sh`
+1. `ci/run.sh` which runs the above scripts in order.
 
 For coverage details locally, please see `ci/out/coverage.html` after running `ci/test.sh`.
 
 See [ci/image/Dockerfile](ci/image/Dockerfile) for the installation of the CI dependencies on Ubuntu.
 
-You can also run tests normally with `go test` once you have the
-[Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite)
-installed. `ci/test.sh` just passes a default set of flags to `go test` to collect coverage,
+You can also run tests normally with `go test`.
+`ci/test.sh` just passes a default set of flags to `go test` to collect coverage,
 enable the race detector, run benchmarks and also prettifies the output.
+
+If you pass flags to `ci/test.sh`, it will pass those flags directly to `go test` but will also
+collect coverage for you. This is nice for when you don't want to wait for benchmarks
+or the race detector but want to have coverage.
+
+Coverage percentage from codecov and the CI scripts will be different because they are calculated differently.

example_echo_test.go

@@ -68,7 +68,7 @@ func Example_echo() {
 func echoServer(w http.ResponseWriter, r *http.Request) error {
 	log.Printf("serving %v", r.RemoteAddr)
 
-	c, err := websocket.Accept(w, r, websocket.AcceptOptions{
+	c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
 		Subprotocols: []string{"echo"},
 	})
 	if err != nil {
@@ -128,7 +128,7 @@ func client(url string) error {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
 
-	c, _, err := websocket.Dial(ctx, url, websocket.DialOptions{
+	c, _, err := websocket.Dial(ctx, url, &websocket.DialOptions{
 		Subprotocols: []string{"echo"},
 	})
 	if err != nil {

example_test.go

@@ -14,7 +14,7 @@ import (
 // message from the client and then closes the connection.
 func ExampleAccept() {
 	fn := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		c, err := websocket.Accept(w, r, websocket.AcceptOptions{})
+		c, err := websocket.Accept(w, r, nil)
 		if err != nil {
 			log.Println(err)
 			return
@@ -46,7 +46,7 @@ func ExampleDial() {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
 
-	c, _, err := websocket.Dial(ctx, "ws://localhost:8080", websocket.DialOptions{})
+	c, _, err := websocket.Dial(ctx, "ws://localhost:8080", nil)
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -64,7 +64,7 @@ func ExampleDial() {
 // on which you will only write and do not expect to read data messages.
 func Example_writeOnly() {
 	fn := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		c, err := websocket.Accept(w, r, websocket.AcceptOptions{})
+		c, err := websocket.Accept(w, r, nil)
 		if err != nil {
 			log.Println(err)
 			return

export_test.go

@@ -1,3 +1,27 @@
 package websocket
 
-var Compute = handleSecWebSocketKey
+import (
+	"context"
+)
+
+type Addr = websocketAddr
+
+const OPClose = opClose
+const OPBinary = opBinary
+const OPPing = opPing
+const OPContinuation = opContinuation
+
+func (c *Conn) WriteFrame(ctx context.Context, fin bool, opcode opcode, p []byte) (int, error) {
+	return c.writeFrame(ctx, fin, opcode, p)
+}
+
+func (c *Conn) WriteHalfFrame(ctx context.Context) (int, error) {
+	return c.realWriteFrame(ctx, header{
+		opcode:        opBinary,
+		payloadLength: 5,
+	}, make([]byte, 10))
+}
+
+func (c *Conn) Flush() error {
+	return c.bw.Flush()
+}

go.mod

@@ -13,8 +13,6 @@ require (
 	golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
 	golang.org/x/tools v0.0.0-20190429184909-35c670923e21
 	golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522
-	gotest.tools/gotestsum v0.3.5
+	gotest.tools/gotestsum v0.3.6-0.20190825182939-fc6cb5870c52
 	mvdan.cc/sh v2.6.4+incompatible
 )
-
-replace gotest.tools/gotestsum => github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b

go.sum

@@ -22,8 +22,6 @@ github.com/mattn/go-colorable v0.0.9 h1:UVL0vNpWh04HeJXV0KLcaT7r06gOH2l4OW6ddYRU
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-isatty v0.0.3 h1:ns/ykhmWi7G9O+8a448SecJU3nSMBXJfqQkl0upE1jI=
 github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
-github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b h1:t6DbmxEtGMM72Uhs638nBOyK7tjsrDwoMfYO1EfQdFE=
-github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b/go.mod h1:Mnf3e5FUzXbkCfynWBGOwLssY7gTQgCHObK9tMpAriY=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.8.0 h1:VkHVNpR4iVnU8XQR6DBm8BqYjN7CRzw+xKUbVVbbW9w=
 github.com/onsi/ginkgo v1.8.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
@@ -85,5 +83,7 @@ gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE=
 gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gotest.tools v2.1.0+incompatible h1:5USw7CrJBYKqjg9R7QlA6jzqZKEAtvW82aNmsxxGPxw=
 gotest.tools v2.1.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
+gotest.tools/gotestsum v0.3.6-0.20190825182939-fc6cb5870c52 h1:Qr31uPFyjpOhAgRfKV4ATUnknnLT2X7HFjqwkstdbbE=
+gotest.tools/gotestsum v0.3.6-0.20190825182939-fc6cb5870c52/go.mod h1:Mnf3e5FUzXbkCfynWBGOwLssY7gTQgCHObK9tMpAriY=
 mvdan.cc/sh v2.6.4+incompatible h1:eD6tDeh0pw+/TOTI1BBEryZ02rD2nMcFsgcvde7jffM=
 mvdan.cc/sh v2.6.4+incompatible/go.mod h1:IeeQbZq+x2SUGBensq/jge5lLQbS3XT2ktyp3wrt4x8=