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 {