From e66fa328e396fe35dfad8ab1e3088ab088aea8be Mon Sep 17 00:00:00 2001 From: Simon Behar Date: Thu, 14 Nov 2019 14:42:51 -0800 Subject: [PATCH] Fixed lint issues (#1739) --- Makefile | 2 +- golangci.yml | 15 +- pkg/apis/workflow/v1alpha1/item_test.go | 29 ++- test/e2e/wait_test.go | 5 +- test/e2e/workflow_test.go | 8 +- util/archive/archive_test.go | 28 ++- util/file/fileutil_test.go | 6 +- .../artifacts/artifactory/artifactory_test.go | 16 +- workflow/artifacts/raw/raw_test.go | 7 +- workflow/common/ancestry_test.go | 8 +- workflow/controller/controller_test.go | 16 +- workflow/controller/operator_persist_test.go | 16 +- workflow/controller/operator_test.go | 188 +++++++++--------- workflow/controller/when_test.go | 4 +- workflow/controller/workflowpod_test.go | 70 +++---- workflow/executor/executor_test.go | 2 +- workflow/util/util_test.go | 25 ++- workflow/validate/validate_dag_test.go | 10 +- workflow/validate/validate_test.go | 49 +++-- 19 files changed, 239 insertions(+), 265 deletions(-) diff --git a/Makefile b/Makefile index 86a91482ce55..10b7f5427e7e 100644 --- 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 index f8646d5571b5..94024fe6a24e 100644 --- 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 index 842f4402c159..896f7359be4a 100644 --- 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 index e375957e7cd0..707e36e98eb6 100644 --- 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 index 4f3292a70f84..728070fa1f52 100644 --- 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 index 2b4766b01fe0..873d0f803066 100644 --- 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 index 3606f2e9936c..b7a40e8527c3 100644 --- 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 index d2f6198dbca3..bfe545215b58 100644 --- 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 index aa1a327fb08b..b616336f2fff 100644 --- a/workflow/artifacts/raw/raw_test.go +++ b/workflow/artifacts/raw/raw_test.go @@ -20,7 +20,7 @@ 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{} @@ -28,10 +28,11 @@ func TestLoad(t *testing.T) { 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 index e9db35a5eba3..3a1b2d28ce81 100644 --- 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 index e4f2e6718bf8..9f6d77b59b40 100644 --- 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 index cc13973280b0..eb565b398bd0 100644 --- 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) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index eb6e15dc0a15..247a12feb68f 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -32,7 +32,7 @@ func TestOperateWorkflowPanicRecover(t *testing.T) { controller.kubeclientset = nil wf := unmarshalWF(helloWorldWf) _, err := controller.wfclientset.ArgoprojV1alpha1().Workflows("").Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() } @@ -82,14 +82,14 @@ func TestSidecarWithVolume(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(sidecarWithVol) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase) pods, err := controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, len(pods.Items) > 0, "pod was not created successfully") pod := pods.Items[0] @@ -128,8 +128,7 @@ func TestProcessNodesWithRetries(t *testing.T) { nodeID := woc.wf.NodeID(nodeName) node := woc.initializeNode(nodeName, wfv1.NodeTypeRetry, &wfv1.Template{}, "", wfv1.NodeRunning) retries := wfv1.RetryStrategy{} - var retryLimit int32 - retryLimit = 2 + retryLimit := int32(2) retries.Limit = &retryLimit woc.wf.Status.Nodes[nodeID] = *node @@ -137,7 +136,7 @@ func TestProcessNodesWithRetries(t *testing.T) { // Ensure there are no child nodes yet. lastChild, err := woc.getLastChildNode(node) - assert.Nil(t, err) + assert.NoError(t, err) assert.Nil(t, lastChild) // Add child nodes. @@ -149,26 +148,27 @@ func TestProcessNodesWithRetries(t *testing.T) { n := woc.getNodeByName(nodeName) lastChild, err = woc.getLastChildNode(n) - assert.Nil(t, err) + assert.NoError(t, err) assert.NotNil(t, lastChild) // Last child is still running. processNodesWithRetries() should return false since // there should be no retries at this point. n, err = woc.processNodeRetries(n, retries) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, n.Phase, wfv1.NodeRunning) // Mark lastChild as successful. woc.markNodePhase(lastChild.Name, wfv1.NodeSucceeded) n, err = woc.processNodeRetries(n, retries) - assert.Nil(t, err) + assert.NoError(t, err) // The parent node also gets marked as Succeeded. assert.Equal(t, n.Phase, wfv1.NodeSucceeded) // Mark the parent node as running again and the lastChild as failed. woc.markNodePhase(n.Name, wfv1.NodeRunning) woc.markNodePhase(lastChild.Name, wfv1.NodeFailed) - woc.processNodeRetries(n, retries) + _, err = woc.processNodeRetries(n, retries) + assert.NoError(t, err) n = woc.getNodeByName(nodeName) assert.Equal(t, n.Phase, wfv1.NodeRunning) @@ -178,7 +178,7 @@ func TestProcessNodesWithRetries(t *testing.T) { woc.addChildNode(nodeName, childNode) n = woc.getNodeByName(nodeName) n, err = woc.processNodeRetries(n, retries) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, n.Phase, wfv1.NodeFailed) } @@ -219,24 +219,24 @@ func TestWorkflowParallelismLimit(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(workflowParallelismLimit) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) // operate again and make sure we don't schedule any more pods makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) // wfBytes, _ := json.MarshalIndent(wf, "", " ") // log.Printf("%s", wfBytes) woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) } @@ -277,25 +277,25 @@ func TestStepsTemplateParallelismLimit(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(stepsTemplateParallelismLimit) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) // operate again and make sure we don't schedule any more pods makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) // wfBytes, _ := json.MarshalIndent(wf, "", " ") // log.Printf("%s", wfBytes) woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) } @@ -333,25 +333,25 @@ func TestDAGTemplateParallelismLimit(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(dagTemplateParallelismLimit) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) // operate again and make sure we don't schedule any more pods makePodsRunning(t, controller.kubeclientset, wf.ObjectMeta.Namespace) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) // wfBytes, _ := json.MarshalIndent(wf, "", " ") // log.Printf("%s", wfBytes) woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) } @@ -426,13 +426,13 @@ func TestNestedTemplateParallelismLimit(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(nestedParallelism) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 4, len(pods.Items)) } @@ -453,11 +453,11 @@ func TestSidecarResourceLimits(t *testing.T) { } wf := unmarshalWF(helloWorldWf) _, err := controller.wfclientset.ArgoprojV1alpha1().Workflows("").Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pod, err := controller.kubeclientset.CoreV1().Pods("").Get("hello-world", metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) var waitCtr *apiv1.Container for _, ctr := range pod.Spec.Containers { if ctr.Name == "wait" { @@ -476,32 +476,32 @@ func TestSuspendResume(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(stepsTemplateParallelismLimit) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) // suspend the workflow err = util.SuspendWorkflow(wfcset, wf.ObjectMeta.Name) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, *wf.Spec.Suspend) // operate should not result in no workflows being created since it is suspended woc := newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 0, len(pods.Items)) // resume the workflow and operate again. two pods should be able to be scheduled err = util.ResumeWorkflow(wfcset, wf.ObjectMeta.Name) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Nil(t, wf.Spec.Suspend) woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 2, len(pods.Items)) } @@ -592,11 +592,11 @@ func TestInputParametersAsJson(t *testing.T) { wf := unmarshalWF(inputParametersAsJson) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() updatedWf, err := wfcset.Get(wf.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) found := false for _, node := range updatedWf.Status.Nodes { if node.Type == wfv1.NodeTypePod { @@ -648,14 +648,14 @@ func TestExpandWithItems(t *testing.T) { // Test list expansion wf := unmarshalWF(expandWithItems) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) newSteps, err := woc.expandStep(wf.Spec.Templates[0].Steps[0].Steps[0]) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 5, len(newSteps)) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 5, len(pods.Items)) } @@ -696,10 +696,10 @@ func TestExpandWithItemsMap(t *testing.T) { wf := unmarshalWF(expandWithItemsMap) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) newSteps, err := woc.expandStep(wf.Spec.Templates[0].Steps[0].Steps[0]) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 3, len(newSteps)) } @@ -735,31 +735,32 @@ func TestSuspendTemplate(t *testing.T) { // operate the workflow. it should become in a suspended state after wf := unmarshalWF(suspendTemplate) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, util.IsWorkflowSuspended(wf)) // operate again and verify no pods were scheduled woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 0, len(pods.Items)) // resume the workflow. verify resume workflow edits nodestatus correctly - util.ResumeWorkflow(wfcset, wf.ObjectMeta.Name) + err = util.ResumeWorkflow(wfcset, wf.ObjectMeta.Name) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, util.IsWorkflowSuspended(wf)) // operate the workflow. it should reach the second step woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 1, len(pods.Items)) } @@ -796,18 +797,18 @@ func TestSuspendResumeAfterTemplate(t *testing.T) { // operate the workflow. it should become in a suspended state after wf := unmarshalWF(suspendResumeAfterTemplate) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, util.IsWorkflowSuspended(wf)) // operate again and verify no pods were scheduled woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 0, len(pods.Items)) // wait 4 seconds @@ -817,7 +818,7 @@ func TestSuspendResumeAfterTemplate(t *testing.T) { woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 1, len(pods.Items)) } @@ -828,18 +829,18 @@ func TestSuspendResumeAfterTemplateNoWait(t *testing.T) { // operate the workflow. it should become in a suspended state after wf := unmarshalWF(suspendResumeAfterTemplate) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, util.IsWorkflowSuspended(wf)) // operate again and verify no pods were scheduled woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 0, len(pods.Items)) // don't wait @@ -848,7 +849,7 @@ func TestSuspendResumeAfterTemplateNoWait(t *testing.T) { woc = newWorkflowOperationCtx(wf, controller) woc.operate() pods, err = controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 0, len(pods.Items)) } @@ -892,12 +893,12 @@ func TestWorkflowSpecParam(t *testing.T) { wf := unmarshalWF(volumeWithParam) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pod, err := controller.kubeclientset.CoreV1().Pods("").Get(wf.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) found := false for _, vol := range pod.Spec.Volumes { if vol.Name == "workdir" { @@ -994,10 +995,10 @@ func TestParamSubstitutionWithArtifact(t *testing.T) { woc := newWoc(*wf) woc.operate() wf, err := woc.controller.wfclientset.ArgoprojV1alpha1().Workflows("").Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, wf.Status.Phase, wfv1.NodeRunning) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, len(pods.Items), 1) } @@ -1006,10 +1007,10 @@ func TestGlobalParamSubstitutionWithArtifact(t *testing.T) { woc := newWoc(*wf) woc.operate() wf, err := woc.controller.wfclientset.ArgoprojV1alpha1().Workflows("").Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, wf.Status.Phase, wfv1.NodeRunning) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, len(pods.Items), 1) } @@ -1115,14 +1116,14 @@ func TestMetadataPassing(t *testing.T) { wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("") wf := unmarshalWF(metadataTemplate) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase) pods, err := controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, len(pods.Items) > 0, "pod was not created successfully") var ( @@ -1194,7 +1195,7 @@ func TestResolveIOPathPlaceholders(t *testing.T) { woc.operate() assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase) pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, len(pods.Items) > 0, "pod was not created successfully") assert.Equal(t, []string{"sh", "-c", "head -n 3 <\"/inputs/text/data\" | tee \"/outputs/text/data\" | wc -l > \"/outputs/actual-lines-count/data\""}, pods.Items[0].Spec.Containers[1].Command) @@ -1224,13 +1225,13 @@ func TestResolvePlaceholdersInOutputValues(t *testing.T) { woc.operate() assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase) pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, len(pods.Items) > 0, "pod was not created successfully") templateString := pods.Items[0].ObjectMeta.Annotations["workflows.argoproj.io/template"] var template wfv1.Template err = json.Unmarshal([]byte(templateString), &template) - assert.Nil(t, err) + assert.NoError(t, err) parameterValue := template.Outputs.Parameters[0].Value assert.NotNil(t, parameterValue) assert.NotEmpty(t, *parameterValue) @@ -1263,13 +1264,13 @@ func TestResolvePodNameInRetries(t *testing.T) { woc.operate() assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase) pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, len(pods.Items) > 0, "pod was not created successfully") templateString := pods.Items[0].ObjectMeta.Annotations["workflows.argoproj.io/template"] var template wfv1.Template err = json.Unmarshal([]byte(templateString), &template) - assert.Nil(t, err) + assert.NoError(t, err) parameterValue := template.Outputs.Parameters[0].Value fmt.Println(parameterValue) assert.NotNil(t, parameterValue) @@ -1323,7 +1324,7 @@ func TestResolveStatuses(t *testing.T) { // operate the workflow. it should create a pod. wf := unmarshalWF(outputStatuses) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) jsonValue, err := json.Marshal(&wf.Spec.Templates[0]) assert.NoError(t, err) @@ -1356,26 +1357,26 @@ func TestResourceTemplate(t *testing.T) { // operate the workflow. it should create a pod. wf := unmarshalWF(resourceTemplate) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, wfv1.NodeRunning, wf.Status.Phase) pod, err := controller.kubeclientset.CoreV1().Pods("").Get("resource-template", metav1.GetOptions{}) - if !assert.Nil(t, err) { + if !assert.NoError(t, err) { t.Fatal(err) } tmplStr := pod.Annotations[common.AnnotationKeyTemplate] tmpl := wfv1.Template{} err = yaml.Unmarshal([]byte(tmplStr), &tmpl) - if !assert.Nil(t, err) { + if !assert.NoError(t, err) { t.Fatal(err) } cm := apiv1.ConfigMap{} err = yaml.Unmarshal([]byte(tmpl.Resource.Manifest), &cm) - if !assert.Nil(t, err) { + if !assert.NoError(t, err) { t.Fatal(err) } assert.Equal(t, "resource-cm", cm.Name) @@ -1445,15 +1446,15 @@ func TestResourceWithOwnerReferenceTemplate(t *testing.T) { // operate the workflow. it should create a pod. wf := unmarshalWF(resourceWithOwnerReferenceTemplate) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, wfv1.NodeRunning, wf.Status.Phase) pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - if !assert.Nil(t, err) { + if !assert.NoError(t, err) { t.Fatal(err) } @@ -1462,12 +1463,12 @@ func TestResourceWithOwnerReferenceTemplate(t *testing.T) { tmplStr := pod.Annotations[common.AnnotationKeyTemplate] tmpl := wfv1.Template{} err = yaml.Unmarshal([]byte(tmplStr), &tmpl) - if !assert.Nil(t, err) { + if !assert.NoError(t, err) { t.Fatal(err) } cm := apiv1.ConfigMap{} err = yaml.Unmarshal([]byte(tmpl.Resource.Manifest), &cm) - if !assert.Nil(t, err) { + if !assert.NoError(t, err) { t.Fatal(err) } objectMetas[cm.Name] = cm.ObjectMeta @@ -1523,7 +1524,7 @@ s3: func TestArtifactRepositoryRef(t *testing.T) { wf := unmarshalWF(artifactRepositoryRef) woc := newWoc(*wf) - woc.controller.kubeclientset.CoreV1().ConfigMaps(wf.ObjectMeta.Namespace).Create( + _, err := woc.controller.kubeclientset.CoreV1().ConfigMaps(wf.ObjectMeta.Namespace).Create( &apiv1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "artifact-repository", @@ -1533,12 +1534,13 @@ func TestArtifactRepositoryRef(t *testing.T) { }, }, ) + assert.NoError(t, err) woc.operate() assert.Equal(t, woc.artifactRepository.S3.Bucket, "my-bucket") assert.Equal(t, woc.artifactRepository.S3.Endpoint, "my-minio-endpoint.default:9000") assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase) pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, len(pods.Items) > 0, "pod was not created successfully") } @@ -1622,13 +1624,13 @@ func TestStepWFGetNodeName(t *testing.T) { // operate the workflow. it should create a pod. wf := unmarshalWF(stepScriptTmpl) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, hasOutputResultRef("generate", &wf.Spec.Templates[0])) assert.False(t, hasOutputResultRef("print-message", &wf.Spec.Templates[0])) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) for _, node := range wf.Status.Nodes { if strings.Contains(node.Name, "generate") { assert.True(t, getStepOrDAGTaskName(node.Name, &wf.Spec.Templates[0].RetryStrategy != nil) == "generate") @@ -1646,13 +1648,13 @@ func TestDAGWFGetNodeName(t *testing.T) { // operate the workflow. it should create a pod. wf := unmarshalWF(dagScriptTmpl) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, hasOutputResultRef("A", &wf.Spec.Templates[0])) assert.False(t, hasOutputResultRef("B", &wf.Spec.Templates[0])) woc := newWorkflowOperationCtx(wf, controller) woc.operate() wf, err = wfcset.Get(wf.ObjectMeta.Name, metav1.GetOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) for _, node := range wf.Status.Nodes { if strings.Contains(node.Name, ".A") { assert.True(t, getStepOrDAGTaskName(node.Name, wf.Spec.Templates[0].RetryStrategy != nil) == "A") @@ -1701,10 +1703,10 @@ func TestWithParamAsJsonList(t *testing.T) { // Test list expansion wf := unmarshalWF(withParamAsJsonList) wf, err := wfcset.Create(wf) - assert.Nil(t, err) + assert.NoError(t, err) woc := newWorkflowOperationCtx(wf, controller) woc.operate() pods, err := controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 4, len(pods.Items)) } diff --git a/workflow/controller/when_test.go b/workflow/controller/when_test.go index eac3b02c68ee..5f52924584ff 100644 --- a/workflow/controller/when_test.go +++ b/workflow/controller/when_test.go @@ -25,7 +25,7 @@ func TestShouldExecute(t *testing.T) { } for _, trueExp := range trueExpressions { res, err := shouldExecute(trueExp) - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, res) } @@ -47,7 +47,7 @@ func TestShouldExecute(t *testing.T) { } for _, falseExp := range falseExpressions { res, err := shouldExecute(falseExp) - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, res) } } diff --git a/workflow/controller/workflowpod_test.go b/workflow/controller/workflowpod_test.go index 2d5aa7ca9fa8..b84ca6ca8fad 100644 --- a/workflow/controller/workflowpod_test.go +++ b/workflow/controller/workflowpod_test.go @@ -42,18 +42,6 @@ func newWoc(wfs ...wfv1.Workflow) *wfOperationCtx { return woc } -// getPodName returns the podname of the created pod of a workflow -// Only applies to single pod workflows -func getPodName(wf *wfv1.Workflow) string { - if len(wf.Status.Nodes) != 1 { - panic("getPodName called against a multi-pod workflow") - } - for podName := range wf.Status.Nodes { - return podName - } - return "" -} - var scriptTemplateWithInputArtifact = ` name: script-with-input-artifact inputs: @@ -145,7 +133,7 @@ func TestWFLevelServiceAccount(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, pod.Spec.ServiceAccountName, "foo") @@ -160,7 +148,7 @@ func TestTmplServiceAccount(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, pod.Spec.ServiceAccountName, "tmpl") @@ -178,7 +166,7 @@ func TestWFLevelAutomountServiceAccountToken(t *testing.T) { err = woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, *pod.Spec.AutomountServiceAccountToken, false) @@ -198,7 +186,7 @@ func TestTmplLevelAutomountServiceAccountToken(t *testing.T) { err = woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, *pod.Spec.AutomountServiceAccountToken, false) @@ -224,7 +212,7 @@ func TestWFLevelExecutorServiceAccountName(t *testing.T) { err = woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, "exec-sa-token", pod.Spec.Volumes[2].Name) @@ -247,7 +235,7 @@ func TestTmplLevelExecutorServiceAccountName(t *testing.T) { err = woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, "exec-sa-token", pod.Spec.Volumes[2].Name) @@ -268,7 +256,7 @@ func TestImagePullSecrets(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, pod.Spec.ImagePullSecrets[0].Name, "secret-name") @@ -300,7 +288,7 @@ func TestAffinity(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.Spec.Affinity) @@ -317,7 +305,7 @@ func TestTolerations(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.Spec.Tolerations) @@ -330,7 +318,7 @@ func TestMetadata(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.ObjectMeta) @@ -355,7 +343,7 @@ func TestWorkflowControllerArchiveConfig(t *testing.T) { } woc.operate() pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) } @@ -380,7 +368,7 @@ func TestWorkflowControllerArchiveConfigUnresolvable(t *testing.T) { } woc.operate() pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 0) } @@ -395,7 +383,7 @@ func TestConditionalNoAddArchiveLocation(t *testing.T) { } woc.operate() pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] var tmpl wfv1.Template @@ -424,7 +412,7 @@ func TestConditionalArchiveLocation(t *testing.T) { } woc.operate() pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] var tmpl wfv1.Template @@ -460,7 +448,7 @@ func TestVolumeAndVolumeMounts(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, 3, len(pod.Spec.Volumes)) @@ -481,7 +469,7 @@ func TestVolumeAndVolumeMounts(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, 2, len(pod.Spec.Volumes)) @@ -501,7 +489,7 @@ func TestVolumeAndVolumeMounts(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, 2, len(pod.Spec.Volumes)) @@ -546,7 +534,7 @@ func TestVolumesPodSubstitution(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, 3, len(pod.Spec.Volumes)) @@ -582,7 +570,7 @@ func TestOutOfCluster(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, "kubeconfig", pod.Spec.Volumes[1].Name) @@ -605,7 +593,7 @@ func TestOutOfCluster(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, "kube-config-secret", pod.Spec.Volumes[1].Name) @@ -626,7 +614,7 @@ func TestPriority(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, pod.Spec.PriorityClassName, "foo") @@ -640,7 +628,7 @@ func TestSchedulerName(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, pod.Spec.SchedulerName, "foo") @@ -692,7 +680,7 @@ func TestInitContainers(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, 1, len(pod.Spec.InitContainers)) @@ -751,7 +739,7 @@ func TestSidecars(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.Equal(t, 3, len(pod.Spec.Containers)) @@ -803,7 +791,7 @@ func TestTemplateLocalVolumes(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] for _, v := range volumes { @@ -824,7 +812,7 @@ func TestWFLevelHostAliases(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.Spec.HostAliases) @@ -841,7 +829,7 @@ func TestTmplLevelHostAliases(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.Spec.HostAliases) @@ -858,7 +846,7 @@ func TestWFLevelSecurityContext(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.Spec.SecurityContext) @@ -875,7 +863,7 @@ func TestTmplLevelSecurityContext(t *testing.T) { err := woc.executeContainer(woc.wf.Spec.Entrypoint, &woc.wf.Spec.Templates[0], "") assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(metav1.ListOptions{}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, pods.Items, 1) pod := pods.Items[0] assert.NotNil(t, pod.Spec.SecurityContext) diff --git a/workflow/executor/executor_test.go b/workflow/executor/executor_test.go index 8d68e72e8cbd..1974c599c839 100644 --- a/workflow/executor/executor_test.go +++ b/workflow/executor/executor_test.go @@ -45,7 +45,7 @@ func TestSaveParameters(t *testing.T) { } mockRuntimeExecutor.On("GetFileContents", fakeContainerID, "/path").Return("has a newline\n", nil) err := we.SaveParameters() - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, *we.Template.Outputs.Parameters[0].Value, "has a newline") } diff --git a/workflow/util/util_test.go b/workflow/util/util_test.go index f10b365bda52..e2a9884d9bdc 100644 --- a/workflow/util/util_test.go +++ b/workflow/util/util_test.go @@ -37,7 +37,7 @@ spec: newWf := wf.DeepCopy() wfClientSet := fakeClientset.NewSimpleClientset() newWf, err := SubmitWorkflow(nil, wfClientSet, "test-namespace", newWf, &SubmitOpts{DryRun: true}) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, wf.Spec, newWf.Spec) assert.Equal(t, wf.Status, newWf.Status) } @@ -61,7 +61,7 @@ func TestResubmitWorkflowWithOnExit(t *testing.T) { Phase: wfv1.NodeSucceeded, } newWF, err := FormulateResubmitWorkflow(&wf, true) - assert.Nil(t, err) + assert.NoError(t, err) newWFOnExitName := newWF.ObjectMeta.Name + ".onExit" newWFOneExitID := newWF.NodeID(newWFOnExitName) _, ok := newWF.Status.Nodes[newWFOneExitID] @@ -103,7 +103,7 @@ func TestReadFromSingleorMultiplePath(t *testing.T) { } body, err := ReadFromFilePathsOrUrls(filePaths...) assert.Equal(t, len(body), len(filePaths)) - assert.Nil(t, err) + assert.NoError(t, err) for i := range body { assert.Equal(t, body[i], []byte(tc.contents[i])) } @@ -173,25 +173,28 @@ func unmarshalWF(yamlStr string) *wfv1.Workflow { var yamlStr = ` containers: - name: main - resources: - limits: - cpu: 1000m + resources: + limits: + cpu: 1000m ` func TestPodSpecPatchMerge(t *testing.T) { tmpl := wfv1.Template{PodSpecPatch: "{\"containers\":[{\"name\":\"main\", \"resources\":{\"limits\":{\"cpu\": \"1000m\"}}}]}"} wf := wfv1.Workflow{Spec: wfv1.WorkflowSpec{PodSpecPatch: "{\"containers\":[{\"name\":\"main\", \"resources\":{\"limits\":{\"memory\": \"100Mi\"}}}]}"}} - merged, _ := PodSpecPatchMerge(&wf, &tmpl) + merged, err := PodSpecPatchMerge(&wf, &tmpl) + assert.NoError(t, err) var spec v1.PodSpec - json.Unmarshal([]byte(merged), &spec) + err = json.Unmarshal([]byte(merged), &spec) + assert.NoError(t, err) assert.Equal(t, "1.000", spec.Containers[0].Resources.Limits.Cpu().AsDec().String()) assert.Equal(t, "104857600", spec.Containers[0].Resources.Limits.Memory().AsDec().String()) tmpl = wfv1.Template{PodSpecPatch: yamlStr} wf = wfv1.Workflow{Spec: wfv1.WorkflowSpec{PodSpecPatch: "{\"containers\":[{\"name\":\"main\", \"resources\":{\"limits\":{\"memory\": \"100Mi\"}}}]}"}} - merged, _ = PodSpecPatchMerge(&wf, &tmpl) - json.Unmarshal([]byte(merged), &spec) + merged, err = PodSpecPatchMerge(&wf, &tmpl) + assert.NoError(t, err) + err = json.Unmarshal([]byte(merged), &spec) + assert.NoError(t, err) assert.Equal(t, "1.000", spec.Containers[0].Resources.Limits.Cpu().AsDec().String()) assert.Equal(t, "104857600", spec.Containers[0].Resources.Limits.Memory().AsDec().String()) - } diff --git a/workflow/validate/validate_dag_test.go b/workflow/validate/validate_dag_test.go index 6507125ba5d7..47c84cd6f2f2 100644 --- a/workflow/validate/validate_dag_test.go +++ b/workflow/validate/validate_dag_test.go @@ -224,7 +224,7 @@ func TestDAGVariableResolution(t *testing.T) { assert.Contains(t, err.Error(), "failed to resolve {{tasks.A.outputs.parameters.unresolvable}}") } err = validate(dagResolvedVar) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(dagResolvedVarNotAncestor) if assert.NotNil(t, err) { @@ -287,7 +287,7 @@ spec: func TestDAGArtifactResolution(t *testing.T) { err := validate(dagResolvedArt) - assert.Nil(t, err) + assert.NoError(t, err) } var dagStatusReference = ` @@ -521,7 +521,7 @@ spec: func TestDAGStatusReference(t *testing.T) { err := validate(dagStatusReference) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(dagStatusNoFutureReferenceSimple) // Can't reference the status of steps that have not run yet @@ -536,7 +536,7 @@ func TestDAGStatusReference(t *testing.T) { } err = validate(dagStatusPastReferenceChain) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(dagStatusOnlyDirectAncestors) // Can't reference steps that are not direct ancestors of node @@ -619,7 +619,7 @@ spec: func TestDAGTargetSubstitution(t *testing.T) { err := validate(dagTargetSubstitution) - assert.Nil(t, err) + assert.NoError(t, err) } var dagTargetMissingInputParam = ` diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go index 98830bb88b96..ff13ff701443 100644 --- a/workflow/validate/validate_test.go +++ b/workflow/validate/validate_test.go @@ -134,8 +134,7 @@ spec: ` func TestDuplicateOrEmptyNames(t *testing.T) { - var err error - err = validate(dupTemplateNames) + err := validate(dupTemplateNames) if assert.NotNil(t, err) { assert.Contains(t, err.Error(), "not unique") } @@ -238,7 +237,7 @@ spec: func TestResolveIOArtifactPathPlaceholders(t *testing.T) { err := validate(ioArtifactPaths) - assert.Nil(t, err) + assert.NoError(t, err) } var outputParameterPath = ` @@ -262,7 +261,7 @@ spec: func TestResolveOutputParameterPathPlaceholder(t *testing.T) { err := validate(outputParameterPath) - assert.Nil(t, err) + assert.NoError(t, err) } var stepOutputReferences = ` @@ -299,7 +298,7 @@ spec: func TestStepOutputReference(t *testing.T) { err := validate(stepOutputReferences) - assert.Nil(t, err) + assert.NoError(t, err) } @@ -338,7 +337,7 @@ spec: func TestStepStatusReference(t *testing.T) { err := validate(stepStatusReferences) - assert.Nil(t, err) + assert.NoError(t, err) } @@ -436,7 +435,7 @@ spec: func TestStepArtReference(t *testing.T) { err := validate(stepArtReferences) - assert.Nil(t, err) + assert.NoError(t, err) } var unsatisfiedParam = ` @@ -514,7 +513,7 @@ spec: func TestGlobalParam(t *testing.T) { err := validate(globalParam) - assert.Nil(t, err) + assert.NoError(t, err) } var invalidTemplateNames = ` @@ -951,7 +950,7 @@ func TestExitHandler(t *testing.T) { // ensure {{workflow.status}} is available in exit handler err = validate(exitHandlerWorkflowStatusOnExit) - assert.Nil(t, err) + assert.NoError(t, err) } var workflowWithPriority = ` @@ -972,7 +971,7 @@ spec: func TestPriorityVariable(t *testing.T) { err := validate(workflowWithPriority) - assert.Nil(t, err) + assert.NoError(t, err) } @@ -1019,7 +1018,7 @@ func TestVolumeMountArtifactPathCollision(t *testing.T) { // tweak the mount path and validation should now be successful wf.Spec.Templates[0].Container.VolumeMounts[0].MountPath = "/differentpath" err = ValidateWorkflow(wfClientset, namespace, wf, ValidateOpts{}) - assert.Nil(t, err) + assert.NoError(t, err) } var activeDeadlineSeconds = ` @@ -1261,7 +1260,7 @@ spec: func TestValidWithItems(t *testing.T) { err := validate(validWithItems) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(invalidWithItems) if assert.NotNil(t, err) { @@ -1300,12 +1299,12 @@ spec: func TestPodNameVariable(t *testing.T) { err := validate(podNameVariable) - assert.Nil(t, err) + assert.NoError(t, err) } func TestGlobalParamWithVariable(t *testing.T) { err := ValidateWorkflow(wfClientset, metav1.NamespaceDefault, test.LoadE2EWorkflow("functional/global-outputs-variable.yaml"), ValidateOpts{}) - assert.Nil(t, err) + assert.NoError(t, err) } var specArgumentNoValue = ` @@ -1330,7 +1329,7 @@ spec: func TestSpecArgumentNoValue(t *testing.T) { wf := unmarshalWf(specArgumentNoValue) err := ValidateWorkflow(wfClientset, metav1.NamespaceDefault, wf, ValidateOpts{Lint: true}) - assert.Nil(t, err) + assert.NoError(t, err) err = ValidateWorkflow(wfClientset, metav1.NamespaceDefault, wf, ValidateOpts{}) assert.NotNil(t, err) } @@ -1367,7 +1366,7 @@ spec: func TestSpecArgumentSnakeCase(t *testing.T) { wf := unmarshalWf(specArgumentSnakeCase) err := ValidateWorkflow(wfClientset, metav1.NamespaceDefault, wf, ValidateOpts{Lint: true}) - assert.Nil(t, err) + assert.NoError(t, err) } var specBadSequenceCountAndEnd = ` @@ -1566,7 +1565,7 @@ spec: func TestLocalTemplateRef(t *testing.T) { err := validate(localTemplateRef) - assert.Nil(t, err) + assert.NoError(t, err) } var undefinedLocalTemplateRef = ` @@ -1617,14 +1616,14 @@ spec: func TestWorkflowTemplate(t *testing.T) { err := validateWorkflowTemplate(templateRefTarget) - assert.Nil(t, err) + assert.NoError(t, err) } func TestTemplateRef(t *testing.T) { err := createWorkflowTemplate(templateRefTarget) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(templateRef) - assert.Nil(t, err) + assert.NoError(t, err) } var templateRefNestedTarget = ` @@ -1656,11 +1655,11 @@ spec: func TestNestedTemplateRef(t *testing.T) { err := createWorkflowTemplate(templateRefTarget) - assert.Nil(t, err) + assert.NoError(t, err) err = createWorkflowTemplate(templateRefNestedTarget) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(nestedTemplateRef) - assert.Nil(t, err) + assert.NoError(t, err) } var templateRefNestedLocalTarget = ` @@ -1694,9 +1693,9 @@ spec: func TestNestedLocalTemplateRef(t *testing.T) { err := createWorkflowTemplate(templateRefNestedLocalTarget) - assert.Nil(t, err) + assert.NoError(t, err) err = validate(nestedLocalTemplateRef) - assert.Nil(t, err) + assert.NoError(t, err) } var undefinedTemplateRef = `