From 7a64dc4dbc9bab2d40ffcde4a9a498cafa5f6158 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Tue, 20 Aug 2019 17:47:10 -0400
Subject: [PATCH] Improve CI and add prettier

---
 .circleci/config.yml |  2 ++
 README.md            | 12 ++++++------
 ci/.gitignore        |  1 +
 ci/fmt.sh            | 37 +++++++++++++++++++------------------
 ci/lib.sh            |  4 +---
 ci/lint.sh           |  2 +-
 ci/out/.gitignore    |  1 -
 ci/run.sh            |  6 +++---
 ci/test.sh           | 17 +++++------------
 docs/CONTRIBUTING.md | 20 ++++++++++++++++----
 websocket_test.go    |  4 +---
 11 files changed, 55 insertions(+), 51 deletions(-)
 create mode 100644 ci/.gitignore
 delete mode 100644 ci/out/.gitignore

diff --git a/.circleci/config.yml b/.circleci/config.yml
index b34f651..9daf2b9 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -45,6 +45,8 @@ jobs:
             # Fallback to using the latest cache if no exact match is found.
             - go-
       - run: ./ci/test.sh
+      - store_artifacts:
+          path: out
       - save_cache:
           paths:
             - /go
diff --git a/README.md b/README.md
index d30ee5f..f3bd6a8 100644
--- a/README.md
+++ b/README.md
@@ -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, "")
 })
 ```
@@ -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.
@@ -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.
@@ -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,
diff --git a/ci/.gitignore b/ci/.gitignore
new file mode 100644
index 0000000..1fcb152
--- /dev/null
+++ b/ci/.gitignore
@@ -0,0 +1 @@
+out
diff --git a/ci/fmt.sh b/ci/fmt.sh
index 52ef3fd..2d3ef4f 100755
--- a/ci/fmt.sh
+++ b/ci/fmt.sh
@@ -4,29 +4,30 @@ set -euo pipefail
 cd "$(dirname "${0}")"
 source ./lib.sh
 
-unstaged_files() {
-  git ls-files --other --modified --exclude-standard
-}
+# Unfortunately, this is the only way to ensure go.mod and go.sum are correct.
+# See https://github.com/golang/go/issues/27005
+go list ./... > /dev/null
+go mod tidy
 
-gen() {
-  # Unfortunately, this is the only way to ensure go.mod and go.sum are correct.
-  # See https://github.com/golang/go/issues/27005
-  go list ./... > /dev/null
-  go mod tidy
+go generate ./...
 
-  go generate ./...
-}
+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")
 
-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 .
+unstaged_files() {
+  git ls-files --other --modified --exclude-standard
 }
 
-gen
-fmt
-
-if [[ $CI && $(unstaged_files) != "" ]]; then
+if [[ ${CI:-} && $(unstaged_files) != "" ]]; then
   echo
   echo "Files either need generation or are formatted incorrectly."
   echo "Please run:"
diff --git a/ci/lib.sh b/ci/lib.sh
index 590e790..080ac00 100644
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -2,9 +2,7 @@
 
 set -euo pipefail
 
-# Ensures $CI can be used if it's set or not.
-export CI=${CI:-}
-if [[ $CI ]]; then
+if [[ ${CI:-} ]]; then
   export GOFLAGS=-mod=readonly
   export DEBIAN_FRONTEND=noninteractive
 fi
diff --git a/ci/lint.sh b/ci/lint.sh
index 6565545..ae5642e 100755
--- a/ci/lint.sh
+++ b/ci/lint.sh
@@ -4,7 +4,7 @@ set -euo pipefail
 cd "$(dirname "${0}")"
 source ./lib.sh
 
-if [[ $CI ]]; then
+if [[ ${CI:-} ]]; then
   apt-get update -qq
   apt-get install -qq shellcheck > /dev/null
 fi
diff --git a/ci/out/.gitignore b/ci/out/.gitignore
deleted file mode 100644
index 72e8ffc..0000000
--- a/ci/out/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-*
diff --git a/ci/run.sh b/ci/run.sh
index 53f0427..8bf1905 100755
--- a/ci/run.sh
+++ b/ci/run.sh
@@ -6,6 +6,6 @@ set -euo pipefail
 cd "$(dirname "${0}")"
 source ./lib.sh
 
-./fmt.sh
-./lint.sh
-./test.sh
+./ci/fmt.sh
+./ci/lint.sh
+./ci/test.sh
diff --git a/ci/test.sh b/ci/test.sh
index f08c2dd..d005735 100755
--- a/ci/test.sh
+++ b/ci/test.sh
@@ -4,12 +4,7 @@ set -euo pipefail
 cd "$(dirname "${0}")"
 source ./lib.sh
 
-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
-
-if [[ $CI ]]; then
+if [[ ${CI:-} ]]; then
   apt-get update -qq
   apt-get install -qq python-pip > /dev/null
   # Need to add pip install directory to $PATH.
@@ -17,14 +12,12 @@ if [[ $CI ]]; then
   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 you'd like to modify the args to go test, just run go test directly, this script is meant
+# for running tests at the end to get coverage and test under the race detector.
+go test -race -vet=off -coverprofile=ci/out/coverage.prof -coverpkg=./... ./...
 
-if [[ $CI ]]; then
+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
diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md
index ade2013..951a326 100644
--- a/docs/CONTRIBUTING.md
+++ b/docs/CONTRIBUTING.md
@@ -12,17 +12,29 @@ 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).
 
-See [../ci/lint.sh](../ci/lint.sh) and [../ci/lint.sh](../ci/test.sh) for the
+On submission please check if CI has passed and if not, please correct your code such that it does.
+If necessary, you may run CI locally with the `ci/run.sh` script.
+You'll only need [shellcheck](https://github.com/koalaman/shellcheck#installing).
+
+For coverage details locally, please see `ci/out/coverage.html` after running `ci/run.sh` or `ci/test.sh`.
+For remote coverage, you can use either [codecov](https://codecov.io/gh/nhooyr/websocket) or download the
+`coverage.html` generated by the go tooling as an artifact on CI.
+
+You can also run any of the steps individually. All of them are scripts in the `ci` directory.
+
+See [../ci/lint.sh](../ci/lint.sh) and [../ci/test.sh](../ci/test.sh) for the
 installation of shellcheck and the Autobahn test suite on Debian or Ubuntu.
 
 For Go, please refer to the [offical docs](https://golang.org/doc/install).
 
-You can benchmark the library with `./ci/benchmark.sh`. You only need Go to run that script.
+You can benchmark the library with `./ci/benchmark.sh`. You only need Go to run that script. Benchmark
+profiles generated by that script are also available for every CI job as artifacts.
diff --git a/websocket_test.go b/websocket_test.go
index be592d9..2397709 100644
--- a/websocket_test.go
+++ b/websocket_test.go
@@ -933,9 +933,7 @@ func checkWSTestIndex(t *testing.T, path string) {
 	if failed {
 		path = strings.Replace(path, ".json", ".html", 1)
 		if os.Getenv("CI") == "" {
-			t.Errorf("wstest found failure, please see %q", path)
-		} else {
-			t.Errorf("wstest found failure, please run test.sh locally to see %q", path)
+			t.Errorf("wstest found failure, please see %q (output as an artifact in CI)", path)
 		}
 	}
 }
-- 
GitLab