Skip to content

Improve CI and performance #118

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

Merged
merged 12 commits into from
Aug 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,59 @@ version: 2
jobs:
fmt:
docker:
- image: golang:1
- image: nhooyr/websocket-ci
steps:
- checkout
- restore_cache:
keys:
- go-{{ checksum "go.sum" }}
- go-v3-{{ checksum "go.sum" }}
# Fallback to using the latest cache if no exact match is found.
- go-
- go-v3-
- run: ./ci/fmt.sh
- save_cache:
paths:
- /go
- /root/gopath
- /root/.cache/go-build
key: go-{{ checksum "go.sum" }}
key: go-v3-{{ checksum "go.sum" }}

lint:
docker:
- image: golang:1
- image: nhooyr/websocket-ci
steps:
- checkout
- restore_cache:
keys:
- go-{{ checksum "go.sum" }}
- go-v3-{{ checksum "go.sum" }}
# Fallback to using the latest cache if no exact match is found.
- go-
- go-v3-
- run: ./ci/lint.sh
- save_cache:
paths:
- /go
- /root/gopath
- /root/.cache/go-build
key: go-{{ checksum "go.sum" }}
key: go-v3-{{ checksum "go.sum" }}

test:
docker:
- image: golang:1
- image: nhooyr/websocket-ci
steps:
- checkout
- restore_cache:
keys:
- go-{{ checksum "go.sum" }}
- go-v3-{{ checksum "go.sum" }}
# Fallback to using the latest cache if no exact match is found.
- go-
- go-v3-
- run: ./ci/test.sh
- store_artifacts:
path: ci/out
destination: out
- save_cache:
paths:
- /go
- /root/gopath
- /root/.cache/go-build
key: go-{{ checksum "go.sum" }}
key: go-v3-{{ checksum "go.sum" }}
- store_test_results:
path: ci/out

workflows:
version: 2
Expand Down
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ http.HandlerFunc(func (w http.ResponseWriter, r *http.Request) {

ctx, cancel := context.WithTimeout(r.Context(), time.Second*10)
defer cancel()

var v interface{}
err = wsjson.Read(ctx, c, &v)
if err != nil {
// ...
}

log.Printf("received: %v", v)

c.Close(websocket.StatusNormalClosure, "")
})
```
Expand Down Expand Up @@ -93,7 +93,7 @@ c.Close(websocket.StatusNormalClosure, "")
## Comparison

Before the comparison, I want to point out that both gorilla/websocket and gobwas/ws were
extremely useful in implementing the WebSocket protocol correctly so *big thanks* to the
extremely useful in implementing the WebSocket protocol correctly so _big thanks_ to the
authors of both. In particular, I made sure to go through the issue tracker of gorilla/websocket
to ensure I implemented details correctly and understood how people were using WebSockets in
production.
Expand All @@ -111,7 +111,7 @@ Just compare the godoc of
The API for nhooyr/websocket has been designed such that there is only one way to do things
which makes it easy to use correctly. Not only is the API simpler, the implementation is
only 1700 lines whereas gorilla/websocket is at 3500 lines. That's more code to maintain,
more code to test, more code to document and more surface area for bugs.
more code to test, more code to document and more surface area for bugs.

Moreover, nhooyr/websocket has support for newer Go idioms such as context.Context and
also uses net/http's Client and ResponseWriter directly for WebSocket handshakes.
Expand Down Expand Up @@ -151,7 +151,7 @@ and clarity.

This library is fantastic in terms of performance. The author put in significant
effort to ensure its speed and I have applied as many of its optimizations as
I could into nhooyr/websocket. Definitely check out his fantastic [blog post](https://medium.freecodecamp.org/million-websockets-and-go-cc58418460bb)
I could into nhooyr/websocket. Definitely check out his fantastic [blog post](https://medium.freecodecamp.org/million-websockets-and-go-cc58418460bb)
about performant WebSocket servers.

If you want a library that gives you absolute control over everything, this is the library,
Expand Down
1 change: 1 addition & 0 deletions ci/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
out
16 changes: 0 additions & 16 deletions ci/bench.sh

This file was deleted.

42 changes: 27 additions & 15 deletions ci/fmt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@

set -euo pipefail
cd "$(dirname "${0}")"
source ./lib.sh

unstaged_files() {
git ls-files --other --modified --exclude-standard
}
cd "$(git rev-parse --show-toplevel)"

gen() {
# Unfortunately, this is the only way to ensure go.mod and go.sum are correct.
Expand All @@ -21,17 +17,33 @@ fmt() {
gofmt -w -s .
go run go.coder.com/go-tools/cmd/goimports -w "-local=$(go list -m)" .
go run mvdan.cc/sh/cmd/shfmt -i 2 -w -s -sr .
# shellcheck disable=SC2046
npx prettier \
--write \
--print-width 120 \
--no-semi \
--trailing-comma all \
--loglevel silent \
$(git ls-files "*.yaml" "*.yml" "*.md")
}

unstaged_files() {
git ls-files --other --modified --exclude-standard
}

check() {
if [[ ${CI:-} && $(unstaged_files) != "" ]]; then
echo
echo "Files need generation or are formatted incorrectly."
echo "Please run:"
echo "./ci/fmt.sh"
echo
git status
git diff
exit 1
fi
}

gen
fmt

if [[ $CI && $(unstaged_files) != "" ]]; then
echo
echo "Files either need generation or are formatted incorrectly."
echo "Please run:"
echo "./ci/fmt.sh"
echo
git status
exit 1
fi
check
13 changes: 13 additions & 0 deletions ci/image/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
FROM golang:1

ENV DEBIAN_FRONTEND=noninteractive
ENV GOPATH=/root/gopath
ENV GOFLAGS="-mod=readonly"
ENV PAGER=cat

RUN apt-get update && \
apt-get install -y shellcheck python-pip npm && \
pip2 install autobahntestsuite && \
npm install -g prettier

RUN git config --global color.ui always
8 changes: 8 additions & 0 deletions ci/image/push.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/env bash

set -euo pipefail
cd "$(dirname "${0}")"
cd "$(git rev-parse --show-toplevel)"

docker build -f ./ci/image/Dockerfile -t nhooyr/websocket-ci .
docker push nhooyr/websocket-ci
12 changes: 0 additions & 12 deletions ci/lib.sh

This file was deleted.

9 changes: 2 additions & 7 deletions ci/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@

set -euo pipefail
cd "$(dirname "${0}")"
source ./lib.sh

if [[ $CI ]]; then
apt-get update -qq
apt-get install -qq shellcheck > /dev/null
fi
cd "$(git rev-parse --show-toplevel)"

# shellcheck disable=SC2046
shellcheck -e SC1091 -x $(git ls-files "*.sh")
shellcheck -x $(git ls-files "*.sh")
go vet ./...
go run golang.org/x/lint/golint -set_exit_status ./...
1 change: 0 additions & 1 deletion ci/out/.gitignore

This file was deleted.

8 changes: 4 additions & 4 deletions ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

set -euo pipefail
cd "$(dirname "${0}")"
source ./lib.sh
cd "$(git rev-parse --show-toplevel)"

./fmt.sh
./lint.sh
./test.sh
./ci/fmt.sh
./ci/lint.sh
./ci/test.sh
38 changes: 16 additions & 22 deletions ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,23 @@

set -euo pipefail
cd "$(dirname "${0}")"
source ./lib.sh
cd "$(git rev-parse --show-toplevel)"

echo "This step includes benchmarks for race detection and coverage purposes
but the numbers will be misleading. please see the bench step or ./bench.sh for
more accurate numbers."
echo
mkdir -p ci/out/websocket
testFlags=(
-race
"-vet=off"
"-bench=."
"-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[@]}"

if [[ $CI ]]; then
apt-get update -qq
apt-get install -qq python-pip > /dev/null
# Need to add pip install directory to $PATH.
export PATH="/home/circleci/.local/bin:$PATH"
pip install -qqq autobahntestsuite
fi

go test -race -coverprofile=ci/out/coverage.prof --vet=off -bench=. -coverpkg=./... "$@" ./...
go tool cover -func=ci/out/coverage.prof

if [[ $CI ]]; then
go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html
if [[ ${CI:-} ]]; then
bash <(curl -s https://codecov.io/bash) -f ci/out/coverage.prof
else
go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html

echo
echo "Please open ci/out/coverage.html to see detailed test coverage stats."
fi
21 changes: 15 additions & 6 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,26 @@ Please capitalize the first word in the commit message title.

The commit message title should use the verb tense + phrase that completes the blank in

> This change modifies websocket to ___________
> This change modifies websocket to \_\_\_\_\_\_\_\_\_

Be sure to link to an existing issue if one exists. In general, try creating an issue
before making a PR to get some discussion going and to make sure you do not spend time
on a PR that may be rejected.

Run `ci/run.sh` to test your changes. You'll need [shellcheck](https://github.com/koalaman/shellcheck#installing), the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite) and Go.
You can run tests normally with `go test`.
You'll need the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite).
In the future this dependency will be removed. See [#117](https://github.com/nhooyr/websocket/issues/117).

See [../ci/lint.sh](../ci/lint.sh) and [../ci/lint.sh](../ci/test.sh) for the
installation of shellcheck and the Autobahn test suite on Debian or Ubuntu.
Please ensure CI passes for your changes. If necessary, you may run CI locally.
The various steps are located in `ci/*.sh`.

For Go, please refer to the [offical docs](https://golang.org/doc/install).
`ci/fmt.sh` requires node (specifically prettier).
`ci/lint.sh` requires [shellcheck](https://github.com/koalaman/shellcheck#installing).
`ci/test.sh` requires the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite).
`ci/run.sh` runs everything in the above order and requires all of their dependencies.

You can benchmark the library with `./ci/benchmark.sh`. You only need Go to run that script.
See [../ci/image/Dockerfile](../ci/image/Dockerfile) for the installation of the CI dependencies on Ubuntu.

For CI coverage, you can use either [codecov](https://codecov.io/gh/nhooyr/websocket) or click the
`coverage.html` artifact on the test step in CI.
For coverage details locally, please see `ci/out/coverage.html` after running `ci/run.sh` or `ci/test.sh`.
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ 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
mvdan.cc/sh v2.6.4+incompatible
)

replace gotest.tools/gotestsum => github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b
Loading