Fix the last of the lint errors and add lint checking to presubmits.

This commit adds the golangci-lint and errcheck.txt exclude files, and runs golangci-lint during presubmits. It also fixes the last of the lint errors!
tektoncd · May 15, 2019 · 089a84c · 089a84c
1 parent 4dcc0d9
commit 089a84c
Show file tree

Hide file tree

Showing 22 changed files with 139 additions and 95 deletions.
diff --git a/.errcheck.txt b/.errcheck.txt
@@ -0,0 +1,8 @@
+(*github.com/tektoncd/pipeline/vendor/go.uber.org/zap.SugaredLogger).Sync
+flag.Set
+os.Setenv
+logger.Sync
+fmt.Fprintf
+fmt.Fprintln
+(io.Closer).Close
+updateConfigMap
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,8 @@
+run:
+ build-tags:
+ - e2e
+ skip-dirs:
+ - vendor/
+linters-settings:
+ errcheck:
+ exclude: .errcheck.txt
diff --git a/pkg/client/clientset/versioned/fake/register.go b/pkg/client/clientset/versioned/fake/register.go
diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go
@@ -53,7 +53,7 @@ func TestRecorderOptions(t *testing.T) {
  Pipelines: ps,
  Tasks: ts,
  }
- c, _ := test.SeedTestData(d)
+ c, _ := test.SeedTestData(t, d)
 
  observer, _ := observer.New(zap.InfoLevel)
 

diff --git a/pkg/reconciler/timeout_handler_test.go b/pkg/reconciler/timeout_handler_test.go
@@ -67,7 +67,7 @@ func TestTaskRunCheckTimeouts(t *testing.T) {
  }
  stopCh := make(chan struct{})
  defer close(stopCh)
- c, _ := test.SeedTestData(d)
+ c, _ := test.SeedTestData(t, d)
  observer, _ := observer.New(zap.InfoLevel)
  th := NewTimeoutHandler(c.Kube, c.Pipeline, stopCh, zap.New(observer).Sugar())
  gotCallback := sync.Map{}
@@ -169,7 +169,7 @@ func TestPipelinRunCheckTimeouts(t *testing.T) {
  },
  }},
  }
- c, _ := test.SeedTestData(d)
+ c, _ := test.SeedTestData(t, d)
  stopCh := make(chan struct{})
  observer, _ := observer.New(zap.InfoLevel)
  th := NewTimeoutHandler(c.Kube, c.Pipeline, stopCh, zap.New(observer).Sugar())
@@ -245,7 +245,7 @@ func TestWithNoFunc(t *testing.T) {
  }},
  }
  stopCh := make(chan struct{})
- c, _ := test.SeedTestData(d)
+ c, _ := test.SeedTestData(t, d)
  observer, _ := observer.New(zap.InfoLevel)
  testHandler := NewTimeoutHandler(c.Kube, c.Pipeline, stopCh, zap.New(observer).Sugar())
  defer func() {

diff --git a/pkg/reconciler/v1alpha1/pipelinerun/cancel_test.go b/pkg/reconciler/v1alpha1/pipelinerun/cancel_test.go
@@ -72,7 +72,7 @@ func TestCancelPipelineRun(t *testing.T) {
  PipelineRuns: []*v1alpha1.PipelineRun{tc.pipelineRun},
  TaskRuns: tc.taskRuns,
  }
- c, _ := test.SeedTestData(d)
+ c, _ := test.SeedTestData(t, d)
  err := cancelPipelineRun(tc.pipelineRun, tc.pipelineState, c.Pipeline)
  if err != nil {
  t.Fatal(err)

diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
@@ -49,8 +49,8 @@ func getRunName(pr *v1alpha1.PipelineRun) string {
 // getPipelineRunController returns an instance of the PipelineRun controller/reconciler that has been seeded with
 // d, where d represents the state of the system (existing resources) needed for the test.
 // recorder can be a fake recorder that is used for testing
-func getPipelineRunController(d test.Data, recorder record.EventRecorder) test.TestAssets {
- c, i := test.SeedTestData(d)
+func getPipelineRunController(t *testing.T, d test.Data, recorder record.EventRecorder) test.TestAssets {
+ c, i := test.SeedTestData(t, d)
  observer, logs := observer.New(zap.InfoLevel)
  stopCh := make(chan struct{})
  configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace())
@@ -183,7 +183,7 @@ func TestReconcile(t *testing.T) {
  // create fake recorder for testing
  fr := record.NewFakeRecorder(1)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 
@@ -333,7 +333,7 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) {
 
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
 
  if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.pipelineRun)); err != nil {
@@ -387,7 +387,7 @@ func TestReconcile_InvalidPipelineRunNames(t *testing.T) {
 
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
- testAssets := getPipelineRunController(test.Data{}, fr)
+ testAssets := getPipelineRunController(t, test.Data{}, fr)
  c := testAssets.Controller
  logs := testAssets.Logs
 
@@ -500,7 +500,7 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) {
  // create fake recorder for testing
  fr := record.NewFakeRecorder(1)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 
@@ -598,7 +598,7 @@ func TestReconcileOnCancelledPipelineRun(t *testing.T) {
  // create fake recorder for testing
  fr := record.NewFakeRecorder(1)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 
@@ -646,7 +646,7 @@ func TestReconcileWithTimeout(t *testing.T) {
  // create fake recorder for testing
  fr := record.NewFakeRecorder(2)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 
@@ -701,7 +701,7 @@ func TestReconcileCancelledPipelineRun(t *testing.T) {
  // create fake recorder for testing
  fr := record.NewFakeRecorder(2)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 
@@ -755,7 +755,7 @@ func TestReconcilePropagateLabels(t *testing.T) {
  // create fake recorder for testing
  fr := record.NewFakeRecorder(2)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 
@@ -866,7 +866,7 @@ func TestReconcileWithTimeoutAndRetry(t *testing.T) {
 
  fr := record.NewFakeRecorder(2)
 
- testAssets := getPipelineRunController(d, fr)
+ testAssets := getPipelineRunController(t, d, fr)
  c := testAssets.Controller
  clients := testAssets.Clients
 

diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go
@@ -801,7 +801,7 @@ func TestGetPipelineConditionStatus(t *testing.T) {
  }
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
- c := GetPipelineConditionStatus("somepipelinerun", tc.state, zap.NewNop().Sugar(), &metav1.Time{time.Now()},
+ c := GetPipelineConditionStatus("somepipelinerun", tc.state, zap.NewNop().Sugar(), &metav1.Time{Time: time.Now()},
  nil)
  if c.Status != tc.expectedStatus {
  t.Fatalf("Expected to get status %s but got %s for state %v", tc.expectedStatus, c.Status, tc.state)

diff --git a/pkg/reconciler/v1alpha1/taskrun/cancel_test.go b/pkg/reconciler/v1alpha1/taskrun/cancel_test.go
@@ -82,7 +82,7 @@ func TestCancelTaskRun(t *testing.T) {
  }
 
  observer, _ := observer.New(zap.InfoLevel)
- c, _ := test.SeedTestData(d)
+ c, _ := test.SeedTestData(t, d)
  err := cancelTaskRun(tc.taskRun, c.Kube, zap.New(observer).Sugar())
  if err != nil {
  t.Fatal(err)

diff --git a/pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint_test.go b/pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint_test.go
@@ -215,12 +215,16 @@ func TestGetRemoteEntrypoint(t *testing.T) {
  if r.Method != http.MethodGet {
  t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
  }
- w.Write(mustRawConfigFile(t, img))
+ if _, err := w.Write(mustRawConfigFile(t, img)); err != nil {
+ t.Fatal(err)
+ }
  case manifestPath:
  if r.Method != http.MethodGet {
  t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
  }
- w.Write(mustRawManifest(t, img))
+ if _, err := w.Write(mustRawManifest(t, img)); err != nil {
+ t.Fatal(err)
+ }
  default:
  t.Fatalf("Unexpected path: %v", r.URL.Path)
  }
@@ -284,12 +288,16 @@ func TestGetRemoteEntrypointWithNonDefaultSA(t *testing.T) {
  if r.Method != http.MethodGet {
  t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
  }
- w.Write(mustRawConfigFile(t, img))
+ if _, err := w.Write(mustRawConfigFile(t, img)); err != nil {
+ t.Fatal(err)
+ }
  case manifestPath:
  if r.Method != http.MethodGet {
  t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
  }
- w.Write(mustRawManifest(t, img))
+ if _, err := w.Write(mustRawManifest(t, img)); err != nil {
+ t.Fatal(err)
+ }
  default:
  t.Fatalf("Unexpected path: %v", r.URL.Path)
  }

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go
@@ -278,10 +278,9 @@ func TestVolumeReplacement(t *testing.T) {
  Name: "${name}",
  VolumeSource: corev1.VolumeSource{
  ConfigMap: &corev1.ConfigMapVolumeSource{
- corev1.LocalObjectReference{"${configmapname}"},
- nil,
- nil,
- nil,
+ LocalObjectReference: corev1.LocalObjectReference{
+ Name: "${configmapname}",
+ },
  },
  }},
  },
@@ -295,10 +294,9 @@ func TestVolumeReplacement(t *testing.T) {
  Name: "myname",
  VolumeSource: corev1.VolumeSource{
  ConfigMap: &corev1.ConfigMapVolumeSource{
- corev1.LocalObjectReference{"cfgmapname"},
- nil,
- nil,
- nil,
+ LocalObjectReference: corev1.LocalObjectReference{
+ Name: "cfgmapname",
+ },
  },
  }},
  },
@@ -310,10 +308,7 @@ func TestVolumeReplacement(t *testing.T) {
  Name: "${name}",
  VolumeSource: corev1.VolumeSource{
  Secret: &corev1.SecretVolumeSource{
- "${secretname}",
- nil,
- nil,
- nil,
+ SecretName: "${secretname}",
  },
  }},
  },
@@ -327,10 +322,7 @@ func TestVolumeReplacement(t *testing.T) {
  Name: "mysecret",
  VolumeSource: corev1.VolumeSource{
  Secret: &corev1.SecretVolumeSource{
- "totallysecure",
- nil,
- nil,
- nil,
+ SecretName: "totallysecure",
  },
  }},
  },

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go
@@ -63,7 +63,7 @@ var (
  }
 )
 
-func setUp() {
+func setUp(t *testing.T) {
  logger, _ = logging.NewLogger("", "")
  fakeClient := fakeclientset.NewSimpleClientset()
  sharedInfomer := informers.NewSharedInformerFactory(fakeClient, 0)
@@ -195,7 +195,9 @@ func setUp() {
  },
  }}
  for _, r := range rs {
- pipelineResourceInformer.Informer().GetIndexer().Add(r)
+ if err := pipelineResourceInformer.Informer().GetIndexer().Add(r); err != nil {
+ t.Fatal(err)
+ }
  }
 }
 
@@ -694,7 +696,7 @@ func TestAddResourceToTask(t *testing.T) {
  },
  }} {
  t.Run(c.desc, func(t *testing.T) {
- setUp()
+ setUp(t)
  names.TestingSeed()
  fakekubeclient := fakek8s.NewSimpleClientset()
  got, err := AddInputResource(fakekubeclient, c.task.Name, &c.task.Spec, c.taskRun, pipelineResourceLister, logger)
@@ -901,7 +903,7 @@ func TestStorageInputResource(t *testing.T) {
  }} {
  t.Run(c.desc, func(t *testing.T) {
  names.TestingSeed()
- setUp()
+ setUp(t)
  fakekubeclient := fakek8s.NewSimpleClientset()
  got, err := AddInputResource(fakekubeclient, c.task.Name, &c.task.Spec, c.taskRun, pipelineResourceLister, logger)
  if (err != nil) != c.wantErr {
@@ -1017,7 +1019,7 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) {
  },
  }} {
  t.Run(c.desc, func(t *testing.T) {
- setUp()
+ setUp(t)
  fakekubeclient := fakek8s.NewSimpleClientset(
  &corev1.ConfigMap{
  ObjectMeta: metav1.ObjectMeta{

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go
@@ -35,7 +35,7 @@ var (
  outputpipelineResourceLister listers.PipelineResourceLister
 )
 
-func outputResourceSetup() {
+func outputResourceSetup(t *testing.T) {
  logger, _ = logging.NewLogger("", "")
  fakeClient := fakeclientset.NewSimpleClientset()
  sharedInfomer := informers.NewSharedInformerFactory(fakeClient, 0)
@@ -99,7 +99,9 @@ func outputResourceSetup() {
  }}
 
  for _, r := range rs {
- pipelineResourceInformer.Informer().GetIndexer().Add(r)
+ if err := pipelineResourceInformer.Informer().GetIndexer().Add(r); err != nil {
+ t.Fatal(err)
+ }
  }
 }
 func TestValidOutputResources(t *testing.T) {
@@ -680,7 +682,7 @@ func TestValidOutputResources(t *testing.T) {
  }} {
  t.Run(c.name, func(t *testing.T) {
  names.TestingSeed()
- outputResourceSetup()
+ outputResourceSetup(t)
  fakekubeclient := fakek8s.NewSimpleClientset()
  err := AddOutputResources(fakekubeclient, c.task.Name, &c.task.Spec, c.taskRun, outputpipelineResourceLister, logger)
  if err != nil {
@@ -855,7 +857,7 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) {
  },
  }} {
  t.Run(c.name, func(t *testing.T) {
- outputResourceSetup()
+ outputResourceSetup(t)
  names.TestingSeed()
  fakekubeclient := fakek8s.NewSimpleClientset(
  &corev1.ConfigMap{
@@ -1108,7 +1110,7 @@ func TestInValidOutputResources(t *testing.T) {
  wantErr: true,
  }} {
  t.Run(c.desc, func(t *testing.T) {
- outputResourceSetup()
+ outputResourceSetup(t)
  fakekubeclient := fakek8s.NewSimpleClientset()
  err := AddOutputResources(fakekubeclient, c.task.Name, &c.task.Spec, c.taskRun, outputpipelineResourceLister, logger)
  if (err != nil) != c.wantErr {