Skip to content

Commit 49a2832

Browse files
authored
Merge pull request #118 from nhooyr/fixes
Improve CI and performance
2 parents 80ddbb4 + 451ecab commit 49a2832

19 files changed

+251
-156
lines changed

.circleci/config.yml

+20-15
Original file line numberDiff line numberDiff line change
@@ -2,54 +2,59 @@ version: 2
22
jobs:
33
fmt:
44
docker:
5-
- image: golang:1
5+
- image: nhooyr/websocket-ci
66
steps:
77
- checkout
88
- restore_cache:
99
keys:
10-
- go-{{ checksum "go.sum" }}
10+
- go-v3-{{ checksum "go.sum" }}
1111
# Fallback to using the latest cache if no exact match is found.
12-
- go-
12+
- go-v3-
1313
- run: ./ci/fmt.sh
1414
- save_cache:
1515
paths:
16-
- /go
16+
- /root/gopath
1717
- /root/.cache/go-build
18-
key: go-{{ checksum "go.sum" }}
18+
key: go-v3-{{ checksum "go.sum" }}
1919

2020
lint:
2121
docker:
22-
- image: golang:1
22+
- image: nhooyr/websocket-ci
2323
steps:
2424
- checkout
2525
- restore_cache:
2626
keys:
27-
- go-{{ checksum "go.sum" }}
27+
- go-v3-{{ checksum "go.sum" }}
2828
# Fallback to using the latest cache if no exact match is found.
29-
- go-
29+
- go-v3-
3030
- run: ./ci/lint.sh
3131
- save_cache:
3232
paths:
33-
- /go
33+
- /root/gopath
3434
- /root/.cache/go-build
35-
key: go-{{ checksum "go.sum" }}
35+
key: go-v3-{{ checksum "go.sum" }}
3636

3737
test:
3838
docker:
39-
- image: golang:1
39+
- image: nhooyr/websocket-ci
4040
steps:
4141
- checkout
4242
- restore_cache:
4343
keys:
44-
- go-{{ checksum "go.sum" }}
44+
- go-v3-{{ checksum "go.sum" }}
4545
# Fallback to using the latest cache if no exact match is found.
46-
- go-
46+
- go-v3-
4747
- run: ./ci/test.sh
48+
- store_artifacts:
49+
path: ci/out
50+
destination: out
4851
- save_cache:
4952
paths:
50-
- /go
53+
- /root/gopath
5154
- /root/.cache/go-build
52-
key: go-{{ checksum "go.sum" }}
55+
key: go-v3-{{ checksum "go.sum" }}
56+
- store_test_results:
57+
path: ci/out
5358

5459
workflows:
5560
version: 2

README.md

+6-6
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ http.HandlerFunc(func (w http.ResponseWriter, r *http.Request) {
4242

4343
ctx, cancel := context.WithTimeout(r.Context(), time.Second*10)
4444
defer cancel()
45-
45+
4646
var v interface{}
4747
err = wsjson.Read(ctx, c, &v)
4848
if err != nil {
4949
// ...
5050
}
51-
51+
5252
log.Printf("received: %v", v)
53-
53+
5454
c.Close(websocket.StatusNormalClosure, "")
5555
})
5656
```
@@ -93,7 +93,7 @@ c.Close(websocket.StatusNormalClosure, "")
9393
## Comparison
9494

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

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

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

157157
If you want a library that gives you absolute control over everything, this is the library,

ci/.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
out

ci/bench.sh

-16
This file was deleted.

ci/fmt.sh

+27-15
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,7 @@
22

33
set -euo pipefail
44
cd "$(dirname "${0}")"
5-
source ./lib.sh
6-
7-
unstaged_files() {
8-
git ls-files --other --modified --exclude-standard
9-
}
5+
cd "$(git rev-parse --show-toplevel)"
106

117
gen() {
128
# Unfortunately, this is the only way to ensure go.mod and go.sum are correct.
@@ -21,17 +17,33 @@ fmt() {
2117
gofmt -w -s .
2218
go run go.coder.com/go-tools/cmd/goimports -w "-local=$(go list -m)" .
2319
go run mvdan.cc/sh/cmd/shfmt -i 2 -w -s -sr .
20+
# shellcheck disable=SC2046
21+
npx prettier \
22+
--write \
23+
--print-width 120 \
24+
--no-semi \
25+
--trailing-comma all \
26+
--loglevel silent \
27+
$(git ls-files "*.yaml" "*.yml" "*.md")
28+
}
29+
30+
unstaged_files() {
31+
git ls-files --other --modified --exclude-standard
32+
}
33+
34+
check() {
35+
if [[ ${CI:-} && $(unstaged_files) != "" ]]; then
36+
echo
37+
echo "Files need generation or are formatted incorrectly."
38+
echo "Please run:"
39+
echo "./ci/fmt.sh"
40+
echo
41+
git status
42+
git diff
43+
exit 1
44+
fi
2445
}
2546

2647
gen
2748
fmt
28-
29-
if [[ $CI && $(unstaged_files) != "" ]]; then
30-
echo
31-
echo "Files either need generation or are formatted incorrectly."
32-
echo "Please run:"
33-
echo "./ci/fmt.sh"
34-
echo
35-
git status
36-
exit 1
37-
fi
49+
check

ci/image/Dockerfile

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
FROM golang:1
2+
3+
ENV DEBIAN_FRONTEND=noninteractive
4+
ENV GOPATH=/root/gopath
5+
ENV GOFLAGS="-mod=readonly"
6+
ENV PAGER=cat
7+
8+
RUN apt-get update && \
9+
apt-get install -y shellcheck python-pip npm && \
10+
pip2 install autobahntestsuite && \
11+
npm install -g prettier
12+
13+
RUN git config --global color.ui always

ci/image/push.sh

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/usr/bin/env bash
2+
3+
set -euo pipefail
4+
cd "$(dirname "${0}")"
5+
cd "$(git rev-parse --show-toplevel)"
6+
7+
docker build -f ./ci/image/Dockerfile -t nhooyr/websocket-ci .
8+
docker push nhooyr/websocket-ci

ci/lib.sh

-12
This file was deleted.

ci/lint.sh

+2-7
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@
22

33
set -euo pipefail
44
cd "$(dirname "${0}")"
5-
source ./lib.sh
6-
7-
if [[ $CI ]]; then
8-
apt-get update -qq
9-
apt-get install -qq shellcheck > /dev/null
10-
fi
5+
cd "$(git rev-parse --show-toplevel)"
116

127
# shellcheck disable=SC2046
13-
shellcheck -e SC1091 -x $(git ls-files "*.sh")
8+
shellcheck -x $(git ls-files "*.sh")
149
go vet ./...
1510
go run golang.org/x/lint/golint -set_exit_status ./...

ci/out/.gitignore

-1
This file was deleted.

ci/run.sh

+4-4
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

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

9-
./fmt.sh
10-
./lint.sh
11-
./test.sh
9+
./ci/fmt.sh
10+
./ci/lint.sh
11+
./ci/test.sh

ci/test.sh

+16-22
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,23 @@
22

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

7-
echo "This step includes benchmarks for race detection and coverage purposes
8-
but the numbers will be misleading. please see the bench step or ./bench.sh for
9-
more accurate numbers."
10-
echo
7+
mkdir -p ci/out/websocket
8+
testFlags=(
9+
-race
10+
"-vet=off"
11+
"-bench=."
12+
"-coverprofile=ci/out/coverage.prof"
13+
"-coverpkg=./..."
14+
)
15+
# https://circleci.com/docs/2.0/collect-test-data/
16+
go run gotest.tools/gotestsum \
17+
--junitfile ci/out/websocket/testReport.xml \
18+
--format=short-verbose \
19+
-- "${testFlags[@]}"
1120

12-
if [[ $CI ]]; then
13-
apt-get update -qq
14-
apt-get install -qq python-pip > /dev/null
15-
# Need to add pip install directory to $PATH.
16-
export PATH="/home/circleci/.local/bin:$PATH"
17-
pip install -qqq autobahntestsuite
18-
fi
19-
20-
go test -race -coverprofile=ci/out/coverage.prof --vet=off -bench=. -coverpkg=./... "$@" ./...
21-
go tool cover -func=ci/out/coverage.prof
22-
23-
if [[ $CI ]]; then
21+
go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html
22+
if [[ ${CI:-} ]]; then
2423
bash <(curl -s https://codecov.io/bash) -f ci/out/coverage.prof
25-
else
26-
go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html
27-
28-
echo
29-
echo "Please open ci/out/coverage.html to see detailed test coverage stats."
3024
fi

docs/CONTRIBUTING.md

+15-6
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,26 @@ Please capitalize the first word in the commit message title.
1212

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

15-
> This change modifies websocket to ___________
15+
> This change modifies websocket to \_\_\_\_\_\_\_\_\_
1616
1717
Be sure to link to an existing issue if one exists. In general, try creating an issue
1818
before making a PR to get some discussion going and to make sure you do not spend time
1919
on a PR that may be rejected.
2020

21-
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.
21+
You can run tests normally with `go test`.
22+
You'll need the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite).
23+
In the future this dependency will be removed. See [#117](https://github.com/nhooyr/websocket/issues/117).
2224

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

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

28-
You can benchmark the library with `./ci/benchmark.sh`. You only need Go to run that script.
33+
See [../ci/image/Dockerfile](../ci/image/Dockerfile) for the installation of the CI dependencies on Ubuntu.
34+
35+
For CI coverage, you can use either [codecov](https://codecov.io/gh/nhooyr/websocket) or click the
36+
`coverage.html` artifact on the test step in CI.
37+
For coverage details locally, please see `ci/out/coverage.html` after running `ci/run.sh` or `ci/test.sh`.

go.mod

+3
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,8 @@ require (
1313
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
1414
golang.org/x/tools v0.0.0-20190429184909-35c670923e21
1515
golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522
16+
gotest.tools/gotestsum v0.3.5
1617
mvdan.cc/sh v2.6.4+incompatible
1718
)
19+
20+
replace gotest.tools/gotestsum => github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b

0 commit comments

Comments
 (0)