Fixed lint issues (#1739)

argoproj · Nov 14, 2019 · e66fa32 · e66fa32
1 parent 63e12d0
commit e66fa32
Show file tree

Hide file tree

Showing 19 changed files with 239 additions and 265 deletions.
diff --git a/Makefile b/Makefile
@@ -130,7 +130,7 @@ endif
 .PHONY: lint
 lint:
 ifdef GOLANGCI_EXISTS
-	golangci-lint run --config golangci.yml
+	golangci-lint run --fix --verbose --config golangci.yml
 else
 	# Remove gometalinter after a migration time.
 	gometalinter --config gometalinter.json ./...

diff --git a/golangci.yml b/golangci.yml
@@ -1,17 +1,6 @@
 run:
-  deadline: 8m
   tests: false
+  deadline: 30m
   skip-files:
     - "pkg/client"
-  deadline: 10m
-output:
-  format: colored-line-number
-  print-issued-lines: true
-  print-linter-name: true
-linters:
-  enable:
-  - gofmt
-  - goimports
-  - unconvert
-  - misspell
-  fast: false
+    - "vendor/"
diff --git a/pkg/apis/workflow/v1alpha1/item_test.go b/pkg/apis/workflow/v1alpha1/item_test.go
@@ -10,29 +10,28 @@ import (
 
 func TestItem(t *testing.T) {
 	testData := map[string]Type{
-		"0":             Number,
-		"3.141":         Number,
-		"true":          Bool,
-		"\"hello\"":     String,
-		"{\"val\":\"123\"}": Map,
-		"[\"1\",\"2\",\"3\",\"4\",\"5\"]" : List,
+		"0":                               Number,
+		"3.141":                           Number,
+		"true":                            Bool,
+		"\"hello\"":                       String,
+		"{\"val\":\"123\"}":               Map,
+		"[\"1\",\"2\",\"3\",\"4\",\"5\"]": List,
 	}
 
 	for data, expectedType := range testData {
 		var itm Item
 		err := json.Unmarshal([]byte(data), &itm)
-     		assert.Nil(t, err)
+		assert.NoError(t, err)
 		assert.Equal(t, itm.Type, expectedType)
 		jsonBytes, err := json.Marshal(itm)
-		assert.Equal(t, string(data), string(jsonBytes))
+		assert.NoError(t, err)
+		assert.Equal(t, data, string(jsonBytes))
 		if itm.Type == String {
-			assert.Equal(t, string(data), fmt.Sprintf("\"%v\"", itm))
-			assert.Equal(t, string(data), fmt.Sprintf("\"%s\"", itm))
-		}else {
-			assert.Equal(t, string(data), fmt.Sprintf("%v", itm))
-			assert.Equal(t, string(data), fmt.Sprintf("%s", itm))
+			assert.Equal(t, data, fmt.Sprintf("\"%v\"", itm))
+			assert.Equal(t, data, fmt.Sprintf("\"%s\"", itm))
+		} else {
+			assert.Equal(t, data, fmt.Sprintf("%v", itm))
+			assert.Equal(t, data, fmt.Sprintf("%s", itm))
 		}
 	}
 }
-
-
diff --git a/test/e2e/wait_test.go b/test/e2e/wait_test.go
@@ -75,7 +75,10 @@ spec:
 	assert.Equal(t, false, wf.Status.FinishedAt.IsZero())
 
 	deleteOptions := metav1.DeleteOptions{}
-	wfClient.Delete(workflowName, &deleteOptions)
+	err = wfClient.Delete(workflowName, &deleteOptions)
+	if err != nil {
+		log.Fatal(err)
+	}
 }
 
 func TestWaitCmd(t *testing.T) {

diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go
@@ -71,8 +71,7 @@ spec:
 		if err != nil {
 			log.Fatal(err)
 		}
-		var allCompleted bool
-		allCompleted = true
+		allCompleted := true
 		for k, v := range wf.Status.Nodes {
 			if !v.Completed() {
 				fmt.Printf("Status of %s: %v\n", k, v.Phase)
@@ -89,7 +88,10 @@ spec:
 	}
 
 	deleteOptions := metav1.DeleteOptions{}
-	wfClient.Delete(workflowName, &deleteOptions)
+	err = wfClient.Delete(workflowName, &deleteOptions)
+	if err != nil {
+		log.Fatal(err)
+	}
 }
 
 func TestArgoWorkflows(t *testing.T) {

diff --git a/util/archive/archive_test.go b/util/archive/archive_test.go
@@ -16,45 +16,51 @@ func tempFile(dir, prefix, suffix string) (*os.File, error) {
 	if dir == "" {
 		dir = os.TempDir()
 	} else {
-		os.MkdirAll(dir, 0700)
+		err := os.MkdirAll(dir, 0700)
+		if err != nil {
+			return nil, err
+		}
 	}
 	randBytes := make([]byte, 16)
-	rand.Read(randBytes)
+	_, err := rand.Read(randBytes)
+	if err != nil {
+		return nil, err
+	}
 	filePath := filepath.Join(dir, prefix+hex.EncodeToString(randBytes)+suffix)
 	return os.Create(filePath)
 }
 
 func TestTarDirectory(t *testing.T) {
 	f, err := tempFile(os.TempDir()+"/argo-test", "dir-", ".tgz")
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	log.Infof("Taring to %s", f.Name())
 	w := bufio.NewWriter(f)
 
 	err = TarGzToWriter("../../test/e2e", w)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	err = f.Close()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 }
 
 func TestTarFile(t *testing.T) {
 	data, err := tempFile(os.TempDir()+"/argo-test", "file-", "")
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	_, err = data.WriteString("hello world")
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	data.Close()
 
 	dataTarPath := data.Name() + ".tgz"
 	f, err := os.Create(dataTarPath)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	log.Infof("Taring to %s", f.Name())
 	w := bufio.NewWriter(f)
 
 	err = TarGzToWriter(data.Name(), w)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	err = os.Remove(data.Name())
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	err = f.Close()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 }
diff --git a/util/file/fileutil_test.go b/util/file/fileutil_test.go
@@ -41,12 +41,12 @@ func TestExistsInTar(t *testing.T) {
 			}
 			hdr := tar.Header{Name: f.name, Mode: int64(mode), Size: int64(len(f.body))}
 			err := writer.WriteHeader(&hdr)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			_, err = writer.Write([]byte(f.body))
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 		}
 		err := writer.Close()
-		assert.Nil(t, err)
+		assert.NoError(t, err)
 		return tar.NewReader(&buf)
 	}
 

diff --git a/workflow/artifacts/artifactory/artifactory_test.go b/workflow/artifacts/artifactory/artifactory_test.go
@@ -27,18 +27,18 @@ func TestSaveAndLoad(t *testing.T) {
 
 	// create file to test save
 	lf, err := ioutil.TempFile("", LoadFileName)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	defer os.Remove(lf.Name())
 	// load file with test content
 	content := []byte(fileContent)
 	_, err = lf.Write(content)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	err = lf.Close()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// create file to test load
 	sf, err := ioutil.TempFile("", SaveFileName)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	defer os.Remove(sf.Name())
 
 	artL := &wfv1.Artifact{}
@@ -49,10 +49,12 @@ func TestSaveAndLoad(t *testing.T) {
 		Username: Username,
 		Password: Password,
 	}
-	driver.Save(lf.Name(), artL)
-	driver.Load(artL, sf.Name())
+	err = driver.Save(lf.Name(), artL)
+	assert.NoError(t, err)
+	err = driver.Load(artL, sf.Name())
+	assert.NoError(t, err)
 
 	dat, err := ioutil.ReadFile(sf.Name())
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, fileContent, string(dat))
 }
diff --git a/workflow/artifacts/raw/raw_test.go b/workflow/artifacts/raw/raw_test.go
@@ -20,18 +20,19 @@ func TestLoad(t *testing.T) {
 
 	content := "time: " + string(time.Now().UnixNano())
 	lf, err := ioutil.TempFile("", LoadFileName)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	defer os.Remove(lf.Name())
 
 	art := &wfv1.Artifact{}
 	art.Raw = &wfv1.RawArtifact{
 		Data: content,
 	}
 	driver := &raw.RawArtifactDriver{}
-	driver.Load(art, lf.Name())
+	err = driver.Load(art, lf.Name())
+	assert.NoError(t, err)
 
 	dat, err := ioutil.ReadFile(lf.Name())
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, content, string(dat))
 
 }
diff --git a/workflow/common/ancestry_test.go b/workflow/common/ancestry_test.go
@@ -106,16 +106,16 @@ func TestGetTaskAncestryForGlobalArtifacts(t *testing.T) {
 	ctx := &testContext{
 		status: map[string]*wfv1.NodeStatus{
 			"task1": {
-				FinishedAt: v1.Time{time.Now().Add(1 * time.Minute)},
+				FinishedAt: v1.Time{Time: time.Now().Add(1 * time.Minute)},
 			},
 			"task2": {
-				FinishedAt: v1.Time{time.Now().Add(3 * time.Minute)},
+				FinishedAt: v1.Time{Time: time.Now().Add(3 * time.Minute)},
 			},
 			"task3": {
-				FinishedAt: v1.Time{time.Now().Add(2 * time.Minute)},
+				FinishedAt: v1.Time{Time: time.Now().Add(2 * time.Minute)},
 			},
 			"task4": {
-				FinishedAt: v1.Time{time.Now().Add(4 * time.Minute)},
+				FinishedAt: v1.Time{Time: time.Now().Add(4 * time.Minute)},
 			},
 		},
 	}

diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go
@@ -1,22 +1,18 @@
 package controller
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
-	"io"
-	"io/ioutil"
 	"k8s.io/client-go/util/workqueue"
 	"testing"
 	"time"
 
-	"sigs.k8s.io/yaml"
 	"github.com/stretchr/testify/assert"
 	apiv1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/cache"
+	"sigs.k8s.io/yaml"
 
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	fakewfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned/fake"
@@ -67,14 +63,6 @@ func newController() *WorkflowController {
 	}
 }
 
-func marshallBody(b interface{}) io.ReadCloser {
-	result, err := json.Marshal(b)
-	if err != nil {
-		panic(err)
-	}
-	return ioutil.NopCloser(bytes.NewReader(result))
-}
-
 func unmarshalWF(yamlStr string) *wfv1.Workflow {
 	var wf wfv1.Workflow
 	err := yaml.Unmarshal([]byte(yamlStr), &wf)
@@ -88,7 +76,7 @@ func unmarshalWF(yamlStr string) *wfv1.Workflow {
 func makePodsRunning(t *testing.T, kubeclientset kubernetes.Interface, namespace string) {
 	podcs := kubeclientset.CoreV1().Pods(namespace)
 	pods, err := podcs.List(metav1.ListOptions{})
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	for _, pod := range pods.Items {
 		pod.Status.Phase = apiv1.PodRunning
 		_, _ = podcs.Update(&pod)

diff --git a/workflow/controller/operator_persist_test.go b/workflow/controller/operator_persist_test.go
@@ -50,9 +50,7 @@ func TestPersistWithoutLargeWfSupport(t *testing.T) {
 	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
 	wf := unmarshalWF(helloWorldWfPersist)
 	wf, err := wfcset.Create(wf)
-	if err != nil {
-
-	}
+	assert.NoError(t, err)
 	controller.wfDBctx = getMockDBCtx(sqldb.DBUpdateNoRowFoundError(nil, "test"), false, false)
 	woc := newWorkflowOperationCtx(wf, controller)
 	woc.operate()
@@ -66,9 +64,7 @@ func TestPersistErrorWithoutLargeWfSupport(t *testing.T) {
 	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
 	wf := unmarshalWF(helloWorldWfPersist)
 	wf, err := wfcset.Create(wf)
-	if err != nil {
-
-	}
+	assert.NoError(t, err)
 	var err1 error = errors.New("23324", "test")
 	controller.wfDBctx = getMockDBCtx(sqldb.DBUpdateNoRowFoundError(err1, "test"), false, false)
 	woc := newWorkflowOperationCtx(wf, controller)
@@ -84,9 +80,7 @@ func TestPersistWithLargeWfSupport(t *testing.T) {
 	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
 	wf := unmarshalWF(helloWorldWfPersist)
 	wf, err := wfcset.Create(wf)
-	if err != nil {
-
-	}
+	assert.NoError(t, err)
 	controller.wfDBctx = getMockDBCtx(sqldb.DBUpdateNoRowFoundError(nil, "test"), true, true)
 	woc := newWorkflowOperationCtx(wf, controller)
 	woc.operate()
@@ -100,9 +94,7 @@ func TestPersistErrorWithLargeWfSupport(t *testing.T) {
 	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
 	wf := unmarshalWF(helloWorldWfPersist)
 	wf, err := wfcset.Create(wf)
-	if err != nil {
-
-	}
+	assert.NoError(t, err)
 	var err1 error = errors.New("23324", "test")
 	controller.wfDBctx = getMockDBCtx(sqldb.DBUpdateNoRowFoundError(err1, "test"), true, false)
 	woc := newWorkflowOperationCtx(wf, controller)