Merge pull request #158 from form3tech-oss/nvloff-fixes

chore: use golangci-lint & fix formatting

.github/workflows/ci.yaml

@@ -9,5 +9,5 @@ jobs:
           go-version: "1.20"
       - name: test
         run: |
-          make install-goimports
+          make lint
           make test

.gitignore

@@ -1 +1,2 @@
 .idea
+/tools

.golangci.yml

@@ -0,0 +1,108 @@
+# Using [email protected]
+run:
+  timeout: 5m
+  tests: true
+  skip-dirs:
+    - vendor$
+output:
+  print-linter-name: true
+linters:
+  enable-all: true
+  fast: false
+  disable:
+  # Deprecated (see https://golangci-lint.run/usage/linters/)
+  - deadcode
+  - golint
+  - ifshort
+  - interfacer
+  - maligned
+  - nosnakecase
+  - scopelint
+  - structcheck
+  - varcheck
+  - exhaustivestruct
+
+  # Annoying style guides that are very subjective
+  - funlen
+  - nlreturn
+  - wsl
+  - cyclop
+  - varnamelen
+  - maintidx
+  - gocognit
+  - godot
+  - gocyclo
+  - nestif
+  - nilnil
+  - exhaustruct
+
+  # Requires too many changes
+  - testpackage
+
+  - thelper
+  - wrapcheck
+  - testifylint
+
+  # TODO
+  - usestdlibvars
+  - paralleltest
+  - perfsprint
+  - staticcheck
+  - gomnd
+  - nonamedreturns
+  - goerr113
+  - gochecknoglobals
+  - dupword
+  - depguard
+  - unparam
+  - revive
+  - stylecheck
+  - unused
+  - protogetter
+  - lll
+  - prealloc
+  - forcetypeassert
+  - gocritic
+  - forbidigo
+  - dupl
+  - errcheck
+  - gosec
+  - ineffassign
+  - wastedassign
+
+linters-settings:
+  tagliatelle:
+    case:
+      rules:
+        json: snake
+        yaml: kebab
+  depguard:
+    rules:
+      main:
+        deny:
+          - pkg: "github.com/pkg/errors"
+            desc: "Please use go built-in error wrapping and handling via `fmt` and `errors` packages"
+  gci:
+    sections:
+      - standard
+      - default
+      - prefix(github.com/form3tech-oss/f1)
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+  exclude-rules:
+    # exclude long lines with URLs
+    - path: \.go
+      source: https://
+      linters:
+        - lll
+    # Don't wrap check RoundTrip
+    - path: \.go
+      source: RoundTrip
+      linters:
+        - wrapcheck
+    - path: _test\.go
+      linters:
+        # unwrapped errors are ok in tests
+        - wrapcheck
+

Makefile

@@ -1,20 +1,22 @@
-GO_FILES?=$$(find ./ -name '*.go' | grep -v /vendor | grep -v /template/ | grep -v /build/ | grep -v swagger-client)
+GOLANGCI_VERSION := 1.56.2
 
-test: goimportscheck
+.PHONY: test
+test:
 	go test ./... -v -race -failfast -p 1 -mod=readonly
 
-require-travis-env:
-ifndef TRAVIS
-	$(error TRAVIS is undefined)
-endif
+.PHONY: tools/golangci-lint
+tools/golangci-lint:
+	@echo "==> Installing golangci-lint..."
+	@./scripts/install-golangci-lint.sh $(GOLANGCI_VERSION)
 
-install-goimports:
-	@if [ -z "$$(command -v goimports)" ]; then \
-    	go get golang.org/x/tools/cmd/goimports; \
-      fi
+.PHONY: lint
+lint: tools/golangci-lint
+	@echo "==> Running golangci-lint..."
+	@tools/golangci-lint run --timeout 600s
+
+.PHONY: lint-fix
+lint-fix: tools/golangci-lint
+	@echo "==> Running golangci-lint..."
+	@tools/golangci-lint run --timeout 600s --fix
 
-goimports:
-	@goimports -w $(GO_FILES)
 
-goimportscheck:
-	@sh -c "'$(CURDIR)/scripts/goimportscheck.sh'"

internal/chart/chart_cmd.go

@@ -5,17 +5,12 @@ import (
 	"os"
 	"time"
 
-	"github.com/pkg/errors"
+	"github.com/guptarohit/asciigraph"
+	"github.com/spf13/cobra"
+	"github.com/wcharczuk/go-chart/v2"
 
 	"github.com/form3tech-oss/f1/v2/internal/support/errorh"
-
 	"github.com/form3tech-oss/f1/v2/internal/trigger/api"
-
-	"github.com/wcharczuk/go-chart/v2"
-
-	"github.com/guptarohit/asciigraph"
-
-	"github.com/spf13/cobra"
 )
 
 func Cmd(builders []api.Builder) *cobra.Command {
@@ -46,24 +41,24 @@ func chartCmdExecute(t api.Builder) func(cmd *cobra.Command, args []string) erro
 
 		startString, err := cmd.Flags().GetString("chart-start")
 		if err != nil {
-			return errors.Wrap(err, "Invalid chart-start value")
+			return fmt.Errorf("getting flag: %w", err)
 		}
 		start, err := time.Parse(time.RFC3339, startString)
 		if err != nil {
-			return err
+			return fmt.Errorf("parsing start time: %w", err)
 		}
 		duration, err := cmd.Flags().GetDuration("chart-duration")
 		if err != nil {
-			return err
+			return fmt.Errorf("getting flag: %w", err)
 		}
 		filename, err := cmd.Flags().GetString("filename")
 		if err != nil {
-			return err
+			return fmt.Errorf("getting flag: %w", err)
 		}
 
 		trig, err := t.New(cmd.Flags())
 		if err != nil {
-			return err
+			return fmt.Errorf("creating builder: %w", err)
 		}
 
 		if trig.DryRun == nil {
@@ -119,13 +114,13 @@ func chartCmdExecute(t api.Builder) func(cmd *cobra.Command, args []string) erro
 
 		f, err := os.Create(filename)
 		if err != nil {
-			return err
+			return fmt.Errorf("creting file: %w", err)
 		}
 		defer errorh.SafeClose(f)
 
 		err = graph.Render(chart.PNG, f)
 		if err != nil {
-			return err
+			return fmt.Errorf("rendering graph: %w", err)
 		}
 		fmt.Printf("Detailed chart written to %s\n", filename)
 		return nil

internal/chart/chart_cmd_stage_test.go

@@ -5,9 +5,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/form3tech-oss/f1/v2/internal/trigger"
-
 	"github.com/stretchr/testify/assert"
+
+	"github.com/form3tech-oss/f1/v2/internal/trigger"
 )
 
 type ChartTestStage struct {
@@ -28,6 +28,7 @@ func NewChartTestStage(t *testing.T) (*ChartTestStage, *ChartTestStage, *ChartTe
 func (s *ChartTestStage) and() *ChartTestStage {
 	return s
 }
+
 func (s *ChartTestStage) i_execute_the_chart_command() *ChartTestStage {
 	cmd := Cmd(trigger.GetBuilders())
 	cmd.SetArgs(s.args)

internal/chart/chart_cmd_test.go

@@ -16,7 +16,6 @@ func TestChartConstant(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }
 
 func TestChartConstantNoJitter(t *testing.T) {
@@ -30,7 +29,6 @@ func TestChartConstantNoJitter(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }
 
 func TestChartStaged(t *testing.T) {
@@ -44,7 +42,6 @@ func TestChartStaged(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }
 
 func TestChartGaussian(t *testing.T) {
@@ -59,7 +56,6 @@ func TestChartGaussian(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }
 
 func TestChartGaussianWithJitter(t *testing.T) {
@@ -75,7 +71,6 @@ func TestChartGaussianWithJitter(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }
 
 func TestChartRamp(t *testing.T) {
@@ -89,7 +84,6 @@ func TestChartRamp(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }
 
 func TestChartFileConfig(t *testing.T) {
@@ -103,5 +97,4 @@ func TestChartFileConfig(t *testing.T) {
 
 	then.
 		the_command_is_successful()
-
 }

internal/raterun/runner_test_stage.go

@@ -5,9 +5,8 @@ import (
 	"testing"
 	"time"
 
-	"go.uber.org/goleak"
-
 	"github.com/stretchr/testify/assert"
+	"go.uber.org/goleak"
 )
 
 type RatedRunnerStage struct {
@@ -19,6 +18,8 @@ type RatedRunnerStage struct {
 }
 
 func NewRatedRunnerStage(t *testing.T) (*RatedRunnerStage, *RatedRunnerStage, *RatedRunnerStage) {
+	t.Helper()
+
 	stage := RatedRunnerStage{
 		t:        t,
 		funcRuns: make(map[time.Duration]int),
@@ -69,7 +70,7 @@ func (s *RatedRunnerStage) function_ran_times(expectedRuns int) *RatedRunnerStag
 		totalRuns += timesRunForRate
 	}
 
-	assert.Equal(s.t, totalRuns, expectedRuns)
+	assert.Equal(s.t, expectedRuns, totalRuns)
 	return s
 }
 

internal/run/active_scenario.go

@@ -3,12 +3,11 @@ package run
 import (
 	"time"
 
-	"github.com/form3tech-oss/f1/v2/pkg/f1/scenarios"
-
-	"github.com/form3tech-oss/f1/v2/pkg/f1/testing"
+	"github.com/google/uuid"
 
 	"github.com/form3tech-oss/f1/v2/internal/metrics"
-	"github.com/google/uuid"
+	"github.com/form3tech-oss/f1/v2/pkg/f1/scenarios"
+	"github.com/form3tech-oss/f1/v2/pkg/f1/testing"
 )
 
 type ActiveScenario struct {

internal/run/logging.go

@@ -36,7 +36,7 @@ func getLogFilePath(scenario string) string {
 func redirectLoggingToFile(scenario string) string {
 	logFilePath := getLogFilePath(scenario)
 
-	file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
+	file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600)
 	if err == nil {
 		log.StandardLogger().SetOutput(file)
 	} else {

internal/run/logging_test.go

@@ -9,8 +9,7 @@ import (
 )
 
 func TestProvidingCustomLogFilePathWithDirectoryThatDoesExist(t *testing.T) {
-	defer os.Setenv("LOG_FILE_PATH", os.Getenv("LOG_FILE_PATH"))
-	os.Setenv("LOG_FILE_PATH", "/does-not-exist/my-scenario.log")
+	t.Setenv("LOG_FILE_PATH", "/does-not-exist/my-scenario.log")
 	expected := getGeneratedLogFilePath("my-scenario")
 
 	actual := getLogFilePath("my-scenario")
@@ -20,9 +19,8 @@ func TestProvidingCustomLogFilePathWithDirectoryThatDoesExist(t *testing.T) {
 }
 
 func TestProvidingCustomLogFilePathWithDirectoryThatDoesNotExistResultsInGeneratedLogFile(t *testing.T) {
-	defer os.Setenv("LOG_FILE_PATH", os.Getenv("LOG_FILE_PATH"))
 	expected := filepath.Join(os.TempDir(), "my-scenario.log")
-	os.Setenv("LOG_FILE_PATH", expected)
+	t.Setenv("LOG_FILE_PATH", expected)
 
 	actual := getLogFilePath("my-scenario")
 
@@ -31,8 +29,7 @@ func TestProvidingCustomLogFilePathWithDirectoryThatDoesNotExistResultsInGenerat
 }
 
 func TestProvidingCustomLogFilePathWhichIsEmptyResultsInGeneratedLogFile(t *testing.T) {
-	defer os.Setenv("LOG_FILE_PATH", os.Getenv("LOG_FILE_PATH"))
-	os.Setenv("LOG_FILE_PATH", "")
+	t.Setenv("LOG_FILE_PATH", "")
 	expected := getGeneratedLogFilePath("my-scenario")
 
 	actual := getLogFilePath("my-scenario")

internal/run/main_test.go

@@ -12,11 +12,12 @@ import (
 
 var fakePrometheus FakePrometheus
 
-const fakePrometheusNamespace = "test-namespace"
-const fakePrometheusID = "test-run-name"
+const (
+	fakePrometheusNamespace = "test-namespace"
+	fakePrometheusID        = "test-run-name"
+)
 
 func TestMain(m *testing.M) {
-
 	var err error
 	fakePrometheus.Port, err = freeport.GetFreePort()
 	if err != nil {