ci: add Makefile, use golangci-lint

With this commit we add a Makefile with the goal of removing the goclean.sh script. We also use the more powerful golangci-lint linter and split the GitHub actions into multiple sections to parallelize them.
2021-09-27 14:24:40 +02:00 · 2021-09-27 14:24:40 +02:00 · 97c3fdfaa2
commit 97c3fdfaa2
parent 43b172ec93
6 changed files with 220 additions and 133 deletions
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@ -1,31 +1,64 @@
 name: Build and Test
 on: [push, pull_request]
+
+env:
+  # go needs absolute directories, using the $HOME variable doesn't work here.
+  GOCACHE: /home/runner/work/go/pkg/build
+  GOPATH: /home/runner/work/go
+  GO_VERSION: 1.16.8
+
 jobs:
  build:
-    name: Go CI
+    name: Build
    runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        go: [1.14, 1.15]
    steps:
      - name: Set up Go
        uses: actions/setup-go@v2
        with:
-          go-version: ${{ matrix.go }}
+          go-version: ${{ env.GO_VERSION }}
+
      - name: Check out source
        uses: actions/checkout@v2
-      - name: Install Linters
-        run: "curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.31.0"
+
      - name: Build
-        env:
-          GO111MODULE: "on"
-        run: go build ./...
+        run: make build
+
+  test-cover:
+    name: Unit coverage
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
+      - name: Check out source
+        uses: actions/checkout@v2
+
      - name: Test
-        env:
-          GO111MODULE: "on"
-        run: |
-          sh ./goclean.sh
+        run: make unit-cover
+
      - name: Send btcutil coverage
        uses: shogo82148/actions-goveralls@v1
        with:
-          path-to-profile: profile.cov
+          path-to-profile: coverage.txt
+
+      - name: Send btcutil coverage for psbt package
+        uses: shogo82148/actions-goveralls@v1
+        with:
+          path-to-profile: psbt/coverage.txt
+
+  test-race:
+    name: Unit race
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
+      - name: Check out source
+        uses: actions/checkout@v2
+
+      - name: Test
+        run: make unit-race
--- a/.gitignore
+++ b/.gitignore
@ -26,3 +26,5 @@ _cgo_export.*
 _testmain.go

 *.exe 
+coverage.txt
+psbt/coverage.txt
--- a/.golangci.yml
+++ b/.golangci.yml
@ -0,0 +1,54 @@
+run:
+  # timeout for analysis
+  deadline: 10m
+
+linters-settings:
+  govet:
+    # Don't report about shadowed variables
+    check-shadowing: false
+  gofmt:
+    # simplify code: gofmt with `-s` option, true by default
+    simplify: true
+
+linters:
+  enable-all: true
+  disable:
+    # Global variables are used in many places throughout the code base.
+    - gochecknoglobals
+
+    # Some lines are over 80 characters on purpose and we don't want to make them
+    # even longer by marking them as 'nolint'.
+    - lll
+
+    # We don't care (enough) about misaligned structs to lint that.
+    - maligned
+
+    # We have long functions, especially in tests. Moving or renaming those would
+    # trigger funlen problems that we may not want to solve at that time.
+    - funlen
+
+    # Disable for now as we haven't yet tuned the sensitivity to our codebase
+    # yet.  Enabling by default for example, would also force new contributors to
+    # potentially extensively refactor code, when they want to smaller change to
+    # land.
+    - gocyclo
+
+    # Instances of table driven tests that don't pre-allocate shouldn't trigger
+    # the linter.
+    - prealloc
+
+    # Init functions are used by loggers throughout the codebase.
+    - gochecknoinits
+
+    # Explicit types are okay.
+    - interfacer
+
+issues:
+  exclude-rules:
+    # Exclude gosec from running for tests so that tests with weak randomness
+    # (math/rand) will pass the linter.
+    - path: _test\.go
+      linters:
+        - gosec
+        #- errcheck
+        - dupl
--- a/116
+++ b/116
@ -0,0 +1,116 @@
+PKG := github.com/btcsuite/btcutil
+
+LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint
+GOACC_PKG := github.com/ory/go-acc
+GOIMPORTS_PKG := golang.org/x/tools/cmd/goimports
+
+GO_BIN := ${GOPATH}/bin
+LINT_BIN := $(GO_BIN)/golangci-lint
+GOACC_BIN := $(GO_BIN)/go-acc
+
+LINT_COMMIT := v1.18.0
+GOACC_COMMIT := 80342ae2e0fcf265e99e76bcc4efd022c7c3811b
+
+DEPGET := cd /tmp && GO111MODULE=on go get -v
+GOBUILD := GO111MODULE=on go build -v
+GOINSTALL := GO111MODULE=on go install -v
+GOTEST := GO111MODULE=on go test 
+
+GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
+
+RM := rm -f
+CP := cp
+MAKE := make
+XARGS := xargs -L 1
+
+# Linting uses a lot of memory, so keep it under control by limiting the number
+# of workers if requested.
+ifneq ($(workers),)
+LINT_WORKERS = --concurrency=$(workers)
+endif
+
+LINT = $(LINT_BIN) run -v $(LINT_WORKERS)
+
+GREEN := "\\033[0;32m"
+NC := "\\033[0m"
+define print
+	echo $(GREEN)$1$(NC)
+endef
+
+default: build
+
+all: build check
+
+# ============
+# DEPENDENCIES
+# ============
+
+$(LINT_BIN):
+	@$(call print, "Fetching linter")
+	$(DEPGET) $(LINT_PKG)@$(LINT_COMMIT)
+
+$(GOACC_BIN):
+	@$(call print, "Fetching go-acc")
+	$(DEPGET) $(GOACC_PKG)@$(GOACC_COMMIT)
+
+goimports:
+	@$(call print, "Installing goimports.")
+	$(DEPGET) $(GOIMPORTS_PKG)
+
+# ============
+# INSTALLATION
+# ============
+
+build:
+	@$(call print, "Compiling btcutil.")
+	$(GOBUILD) $(PKG)/...
+
+# =======
+# TESTING
+# =======
+
+check: unit
+
+unit:
+	@$(call print, "Running unit tests.")
+	$(GOTEST) ./... -test.timeout=20m
+	cd psbt; $(GOTEST) ./... -test.timeout=20m
+
+unit-cover: $(GOACC_BIN)
+	@$(call print, "Running unit coverage tests.")
+	$(GOACC_BIN) ./...
+	cd psbt; $(GOACC_BIN) ./...
+
+unit-race:
+	@$(call print, "Running unit race tests.")
+	env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(GOTEST) -race -test.timeout=20m ./...
+	cd psbt; env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(GOTEST) -race -test.timeout=20m ./...
+
+# =========
+# UTILITIES
+# =========
+
+fmt: goimports
+	@$(call print, "Fixing imports.")
+	goimports -w $(GOFILES_NOVENDOR)
+	@$(call print, "Formatting source.")
+	gofmt -l -w -s $(GOFILES_NOVENDOR)
+
+lint: $(LINT_BIN)
+	@$(call print, "Linting source.")
+	$(LINT)
+
+clean:
+	@$(call print, "Cleaning source.$(NC)")
+	$(RM) coverage.txt psbt/coverage.txt
+
+.PHONY: all \
+	default \
+	build \
+	check \
+	unit \
+	unit-cover \
+	unit-race \
+	fmt \
+	lint \
+	clean
--- a/goclean.sh
+++ b/goclean.sh
@ -1,46 +0,0 @@
-#!/bin/bash
-# The script does automatic checking on a Go package and its sub-packages, including:
-# 1. gofmt         (http://golang.org/cmd/gofmt/)
-# 2. goimports     (https://github.com/bradfitz/goimports)
-# 3. golint        (https://github.com/golang/lint)
-# 4. go vet        (http://golang.org/cmd/vet)
-# 5. gosimple      (https://github.com/dominikh/go-simple)
-# 6. unconvert     (https://github.com/mdempsky/unconvert)
-# 7. race detector (http://blog.golang.org/race-detector)
-# 8. test coverage (http://blog.golang.org/cover)
-#
-
-set -ex
-
-# Automatic checks
-for i in $(find . -name go.mod -type f -print); do
-  module=$(dirname ${i})
-  echo "==> ${module}"
-
-  MODNAME=$(echo $module | sed -E -e "s/^$ROOTPATHPATTERN//" \
-    -e 's,^/,,' -e 's,/v[0-9]+$,,')
-  if [ -z "$MODNAME" ]; then
-    MODNAME=.
-  fi
-
-  # run tests
-  (cd $MODNAME &&
-    echo "mode: atomic" > profile.cov && \
-    env GORACE=halt_on_error=1 go test -race -covermode=atomic -coverprofile=profile.tmp ./... && \
-    cat profile.tmp | tail -n +2 >> profile.cov && \
-    rm profile.tmp && \
-    go tool cover -func profile.cov
-  )
-
-  # check linters
-  (cd $MODNAME && \
-    go mod download && \
-    golangci-lint run --deadline=10m --disable-all \
-      --enable=gofmt \
-      --enable=goimports \
-      --enable=golint \
-      --enable=govet \
-      --enable=gosimple \
-      --enable=unconvert
-  )
-done
--- a/test_coverage.txt
+++ b/test_coverage.txt
@ -1,72 +0,0 @@
-
-github.com/conformal/btcutil/base58.go	 Base58Decode			 100.00% (20/20)
-github.com/conformal/btcutil/base58.go	 Base58Encode			 100.00% (15/15)
-github.com/conformal/btcutil/block.go	 Block.Tx			 100.00% (12/12)
-github.com/conformal/btcutil/wif.go	 WIF.String			 100.00% (11/11)
-github.com/conformal/btcutil/block.go	 Block.Transactions		 100.00% (11/11)
-github.com/conformal/btcutil/amount.go	 AmountUnit.String		 100.00% (8/8)
-github.com/conformal/btcutil/tx.go	 NewTxFromReader		 100.00% (6/6)
-github.com/conformal/btcutil/block.go	 NewBlockFromBytes		 100.00% (6/6)
-github.com/conformal/btcutil/block.go	 NewBlockFromReader		 100.00% (6/6)
-github.com/conformal/btcutil/address.go	 encodeAddress			 100.00% (6/6)
-github.com/conformal/btcutil/address.go	 newAddressPubKeyHash		 100.00% (5/5)
-github.com/conformal/btcutil/address.go	 newAddressScriptHashFromHash	 100.00% (5/5)
-github.com/conformal/btcutil/tx.go	 Tx.Sha				 100.00% (5/5)
-github.com/conformal/btcutil/block.go	 Block.Sha			 100.00% (5/5)
-github.com/conformal/btcutil/amount.go	 NewAmount			 100.00% (5/5)
-github.com/conformal/btcutil/amount.go	 round				 100.00% (3/3)
-github.com/conformal/btcutil/address.go	 NewAddressScriptHash		 100.00% (2/2)
-github.com/conformal/btcutil/amount.go	 Amount.Format			 100.00% (2/2)
-github.com/conformal/btcutil/tx.go	 NewTxFromBytes			 100.00% (2/2)
-github.com/conformal/btcutil/hash160.go	 calcHash			 100.00% (2/2)
-github.com/conformal/btcutil/address.go	 AddressPubKeyHash.Hash160	 100.00% (1/1)
-github.com/conformal/btcutil/block.go	 OutOfRangeError.Error		 100.00% (1/1)
-github.com/conformal/btcutil/block.go	 Block.MsgBlock			 100.00% (1/1)
-github.com/conformal/btcutil/tx.go	 Tx.MsgTx			 100.00% (1/1)
-github.com/conformal/btcutil/hash160.go	 Hash160			 100.00% (1/1)
-github.com/conformal/btcutil/block.go	 NewBlockFromBlockAndBytes	 100.00% (1/1)
-github.com/conformal/btcutil/block.go	 Block.Height			 100.00% (1/1)
-github.com/conformal/btcutil/block.go	 Block.SetHeight		 100.00% (1/1)
-github.com/conformal/btcutil/block.go	 NewBlock			 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKeyHash.IsForNet	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKey.EncodeAddress	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 NewAddressPubKeyHash		 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKeyHash.EncodeAddress 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKeyHash.ScriptAddress 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKeyHash.String	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 NewAddressScriptHashFromHash	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressScriptHash.EncodeAddress 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressScriptHash.ScriptAddress 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressScriptHash.IsForNet	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressScriptHash.String	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressScriptHash.Hash160	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKey.ScriptAddress	 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKey.IsForNet		 100.00% (1/1)
-github.com/conformal/btcutil/address.go	 AddressPubKey.String		 100.00% (1/1)
-github.com/conformal/btcutil/tx.go	 NewTx				 100.00% (1/1)
-github.com/conformal/btcutil/tx.go	 Tx.SetIndex			 100.00% (1/1)
-github.com/conformal/btcutil/amount.go	 Amount.ToUnit			 100.00% (1/1)
-github.com/conformal/btcutil/tx.go	 Tx.Index			 100.00% (1/1)
-github.com/conformal/btcutil/amount.go	 Amount.String			 100.00% (1/1)
-github.com/conformal/btcutil/amount.go	 Amount.MulF64			 100.00% (1/1)
-github.com/conformal/btcutil/appdata.go	 appDataDir			 92.00% (23/25)
-github.com/conformal/btcutil/block.go	 Block.TxLoc			 88.89% (8/9)
-github.com/conformal/btcutil/block.go	 Block.Bytes			 88.89% (8/9)
-github.com/conformal/btcutil/address.go	 NewAddressPubKey		 87.50% (7/8)
-github.com/conformal/btcutil/address.go	 DecodeAddress			 85.00% (17/20)
-github.com/conformal/btcutil/wif.go	 DecodeWIF			 85.00% (17/20)
-github.com/conformal/btcutil/address.go	 AddressPubKey.serialize	 80.00% (4/5)
-github.com/conformal/btcutil/block.go	 Block.TxSha			 75.00% (3/4)
-github.com/conformal/btcutil/wif.go	 paddedAppend			 66.67% (2/3)
-github.com/conformal/btcutil/wif.go	 NewWIF				 66.67% (2/3)
-github.com/conformal/btcutil/certgen.go	 NewTLSCertPair			 0.00% (0/54)
-github.com/conformal/btcutil/wif.go	 WIF.SerializePubKey		 0.00% (0/4)
-github.com/conformal/btcutil/address.go	 AddressPubKey.AddressPubKeyHash 0.00% (0/3)
-github.com/conformal/btcutil/address.go	 AddressPubKey.Format		 0.00% (0/1)
-github.com/conformal/btcutil/address.go	 AddressPubKey.PubKey		 0.00% (0/1)
-github.com/conformal/btcutil/address.go	 AddressPubKey.SetFormat	 0.00% (0/1)
-github.com/conformal/btcutil/wif.go	 WIF.IsForNet			 0.00% (0/1)
-github.com/conformal/btcutil/appdata.go	 AppDataDir			 0.00% (0/1)
-github.com/conformal/btcutil/net.go	 interfaceAddrs			 0.00% (0/1)
-github.com/conformal/btcutil		 ------------------------------- 75.88% (258/340)
-