Enable most of the golangci checks and fix internal/set tests (#416)

* Enable most of the golangci checks and fix internal/set tests

.golangci.yaml

@@ -148,12 +148,8 @@ linters-settings:
     # See https://go-critic.github.io/overview#checks-overview
     # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
     # By default list of stable checks is used.
-    enabled-checks:
-      - rangeValCopy
-
-    # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
-    disabled-checks:
-      - regexpMust
+    # enabled-checks:
+    #   - rangeValCopy
 
     # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks.
     # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
@@ -194,33 +190,13 @@ issues:
 
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
-    - path: internal[/\\]app[/\\]backendapi[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]app[/\\]backendapi[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]app[/\\]mmlogicapi[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]statestorage[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]expbo[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]logging[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]metrics[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: internal[/\\]set[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-    - path: test[/\\]cmd[/\\]
-      linters: [goimports, gochecknoglobals, errcheck, misspell, govet, unparam, golint, gofmt]
-
-    # Exclude some linters from running on tests files.
+    - path: internal[/\\]config[/\\]
+      linters:
+        - gochecknoglobals
+    # Exclue some linters from running on test files
     - path: _test\.go
       linters:
-        - gocyclo
         - errcheck
-        - dupl
-        - gosec
-
     # The following are allowed global variable patterns.
     # Generally it's ok to have constants or variables that effectively act as constants such as a static logger or flag values.
     # The filters below specify the source code pattern that's allowed when declaring a global

internal/set/set_test.go

@@ -15,68 +15,48 @@
 package set
 
 import (
-	"flag"
 	"fmt"
-	"os"
+	"sort"
 	"testing"
-)
 
-var (
-	a1 [5]string
-	a2 [4]string
-	i  [2]string
-	u  [7]string
-	d  [3]string
+	"github.com/stretchr/testify/assert"
 )
 
 type stringOperation func([]string, []string) []string
 
-func TestMain(m *testing.M) {
-
-	a1 = [5]string{"a", "b", "c", "d", "e"}
-	a2 = [4]string{"b", "c", "f", "g"}
-	i = [2]string{"b", "c"}
-	u = [7]string{"a", "b", "c", "d", "e", "f", "g"}
-	d = [3]string{"a", "d", "e"}
-
-	flag.Parse()
-	exitCode := m.Run()
-	os.Exit(exitCode)
-}
-
 func TestStringOperations(t *testing.T) {
-	t.Run("Difference: identical slices", testStringOperation(Difference, a1[:], a1[:], make([]string, 0)))
-	t.Run("Difference: valid slices", testStringOperation(Difference, a1[:], a2[:], d[:]))
-	t.Run("Difference: remove nil", testStringOperation(Difference, a1[:], nil, a1[:]))
-	t.Run("Difference: remove from nil", testStringOperation(Difference, nil, a2[:], nil))
-	t.Run("Union: valid slices", testStringOperation(Union, a1[:], a2[:], u[:]))
-	t.Run("Union: nil first", testStringOperation(Union, nil, a2[:], a2[:]))
-	t.Run("Union: nil second", testStringOperation(Union, a1[:], nil, a1[:]))
-	t.Run("Intersection: valid slices", testStringOperation(Intersection, a1[:], a2[:], i[:]))
-	t.Run("Intersection: nil first", testStringOperation(Intersection, a1[:], nil, nil))
-	t.Run("Intersection: nil second", testStringOperation(Intersection, nil, a2[:], nil))
-}
+	assert := assert.New(t)
 
-func testStringOperation(thisFunc stringOperation, in1 []string, in2 []string, outputExpected []string) func(*testing.T) {
-	return func(t *testing.T) {
-		t.Logf("func(%-15s , %-15s) = %v", fmt.Sprintf("%v", in1), fmt.Sprintf("%v", in2), outputExpected)
-		outputActual := thisFunc(in1, in2)
+	a1 := []string{"a", "b", "c", "d", "e"}
+	a2 := []string{"b", "c", "f", "g"}
+	i := []string{"b", "c"}
+	u := []string{"a", "b", "c", "d", "e", "f", "g"}
+	d := []string{"a", "d", "e"}
 
-		if len(outputActual) != len(outputExpected) {
-			t.Errorf("Length of output string slice incorrect, got %v, want %v", len(outputActual), len(outputExpected))
-		}
-		for x := 0; x < len(outputActual); x++ {
-			found := false
-			for y := 0; y < len(outputExpected) && found != true; y++ {
-				if outputActual[x] == outputExpected[y] {
-					found = true
-				}
-			}
-			if !found {
-				t.Errorf("Element of output string slice incorrect, no %v in %v", outputActual[x], outputExpected)
+	var setTests = []struct {
+		in1      []string
+		in2      []string
+		expected []string
+		op       stringOperation
+	}{
+		{a1, a1, []string{}, Difference},
+		{a1, a2, d, Difference},
+		{a1, nil, a1, Difference},
+		{nil, a2, []string{}, Difference},
+		{a1, a2, u, Union},
+		{nil, a2, a2, Union},
+		{a1, nil, a1, Union},
+		{a1, a2, i, Intersection},
+		{a1, nil, nil, Intersection},
+		{nil, a2, nil, Intersection},
+	}
 
-			}
-
-		}
+	for i, tt := range setTests {
+		t.Run(fmt.Sprintf("%#v-%d", tt.op, i), func(t *testing.T) {
+			actual := tt.op(tt.in1, tt.in2)
+			sort.Strings(tt.expected)
+			sort.Strings(actual)
+			assert.EqualValues(tt.expected, actual)
+		})
 	}
 }

internal/statestore/public.go

@@ -17,18 +17,10 @@ package statestore
 import (
 	"context"
 
-	"github.com/sirupsen/logrus"
 	"open-match.dev/open-match/internal/config"
 	"open-match.dev/open-match/internal/pb"
 )
 
-var (
-	publicLogger = logrus.WithFields(logrus.Fields{
-		"app":       "openmatch",
-		"component": "statestore.public",
-	})
-)
-
 // Service is a generic interface for talking to a storage backend.
 type Service interface {
 	// CreateTicket creates a new Ticket in the state storage. This method fails if the Ticket already exists.

internal/util/netlistener/listener_holder_test.go

@@ -39,7 +39,7 @@ func TestAddrString(t *testing.T) {
 
 	port, err := lh.Obtain()
 	defer func() {
-		err := port.Close()
+		err = port.Close()
 		if err != nil {
 			t.Errorf("error %s while calling port.Close()", err)
 		}