Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the unparam linter, and fix outstanding issues. #1388

Merged
merged 1 commit into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ linters:
- goimports
- gosec
- gocritic
- unparam
5 changes: 1 addition & 4 deletions pkg/apis/pipeline/v1alpha1/build_gcs_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ func (s *BuildGCSResource) GetInputTaskModifier(ts *TaskSpec, sourcePath string)
Args: args,
}}}

volumes, err := getStorageVolumeSpec(s, *ts)
if err != nil {
return nil, err
}
volumes := getStorageVolumeSpec(s, *ts)

return &InternalTaskModifier{
StepsToPrepend: steps,
Expand Down
10 changes: 2 additions & 8 deletions pkg/apis/pipeline/v1alpha1/gcs_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,7 @@ func (s *GCSResource) GetOutputTaskModifier(ts *TaskSpec, path string) (TaskModi
Env: envVars},
}

volumes, err := getStorageVolumeSpec(s, *ts)
if err != nil {
return nil, err
}
volumes := getStorageVolumeSpec(s, *ts)

return &InternalTaskModifier{
StepsToAppend: []Step{step},
Expand Down Expand Up @@ -156,10 +153,7 @@ func (s *GCSResource) GetInputTaskModifier(ts *TaskSpec, path string) (TaskModif
VolumeMounts: secretVolumeMount,
}}}

volumes, err := getStorageVolumeSpec(s, *ts)
if err != nil {
return nil, err
}
volumes := getStorageVolumeSpec(s, *ts)

return &InternalTaskModifier{
StepsToPrepend: steps,
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1alpha1/storage_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func NewStorageResource(images pipeline.Images, r *PipelineResource) (PipelineSt
return nil, xerrors.Errorf("StoreResource: Cannot create a storage resource without type %s in spec", r.Name)
}

func getStorageVolumeSpec(s PipelineStorageResourceInterface, spec TaskSpec) ([]corev1.Volume, error) {
func getStorageVolumeSpec(s PipelineStorageResourceInterface, spec TaskSpec) []corev1.Volume {
var storageVol []corev1.Volume
mountedSecrets := map[string]string{}

Expand All @@ -85,5 +85,5 @@ func getStorageVolumeSpec(s PipelineStorageResourceInterface, spec TaskSpec) ([]
mountedSecrets[volName] = ""
}
}
return storageVol, nil
return storageVol
}
20 changes: 10 additions & 10 deletions pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ import (
"golang.org/x/xerrors"
)

func run(logger *zap.SugaredLogger, cmd string, args ...string) error {
c := exec.Command(cmd, args...)
func run(logger *zap.SugaredLogger, args ...string) error {
c := exec.Command("git", args...)
var output bytes.Buffer
c.Stderr = &output
c.Stdout = &output
if err := c.Run(); err != nil {
logger.Errorf("Error running %v %v: %v\n%v", cmd, args, err, output.String())
logger.Errorf("Error running git %v: %v\n%v", args, err, output.String())
return err
}
return nil
Expand Down Expand Up @@ -70,29 +70,29 @@ func Fetch(logger *zap.SugaredLogger, revision, path, url string) error {
revision = "master"
}
if path != "" {
if err := run(logger, "git", "init", path); err != nil {
if err := run(logger, "init", path); err != nil {
return err
}
if err := os.Chdir(path); err != nil {
return xerrors.Errorf("Failed to change directory with path %s; err: %w", path, err)
}
} else if err := run(logger, "git", "init"); err != nil {
} else if err := run(logger, "init"); err != nil {
return err
}
trimmedURL := strings.TrimSpace(url)
if err := run(logger, "git", "remote", "add", "origin", trimmedURL); err != nil {
if err := run(logger, "remote", "add", "origin", trimmedURL); err != nil {
return err
}
if err := run(logger, "git", "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil {
if err := run(logger, "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil {
// Fetch can fail if an old commitid was used so try git pull, performing regardless of error
// as no guarantee that the same error is returned by all git servers gitlab, github etc...
if err := run(logger, "git", "pull", "--recurse-submodules=yes", "origin"); err != nil {
if err := run(logger, "pull", "--recurse-submodules=yes", "origin"); err != nil {
logger.Warnf("Failed to pull origin : %s", err)
}
if err := run(logger, "git", "checkout", revision); err != nil {
if err := run(logger, "checkout", revision); err != nil {
return err
}
} else if err := run(logger, "git", "reset", "--hard", "FETCH_HEAD"); err != nil {
} else if err := run(logger, "reset", "--hard", "FETCH_HEAD"); err != nil {
return err
}
logger.Infof("Successfully cloned %s @ %s in path %s", trimmedURL, revision, path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1779,7 +1779,11 @@ func TestResolvedConditionCheck_WithResources(t *testing.T) {
getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil }
getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return nil, nil }
getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, xerrors.New("should not get called") }
getCondition := func(name string) (*v1alpha1.Condition, error) {

// This err result is required to satisfy the type alias on this function, but it triggers
// a false positive in the linter: https://github.com/mvdan/unparam/issues/40
// nolint: unparam
getCondition := func(_ string) (*v1alpha1.Condition, error) {
return condition, nil
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/reconciler/taskrun/resources/input_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ var (
}
)

func setUp(t *testing.T) {
func setUp() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I thought we were using t somewhere in that function…

logger, _ = logging.NewLogger("", "")

rs := []*v1alpha1.PipelineResource{{
Expand Down Expand Up @@ -740,7 +740,7 @@ func TestAddResourceToTask(t *testing.T) {
},
}} {
t.Run(c.desc, func(t *testing.T) {
setUp(t)
setUp()
names.TestingSeed()
fakekubeclient := fakek8s.NewSimpleClientset()
got, err := AddInputResource(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun), logger)
Expand Down Expand Up @@ -923,7 +923,7 @@ func TestStorageInputResource(t *testing.T) {
}} {
t.Run(c.desc, func(t *testing.T) {
names.TestingSeed()
setUp(t)
setUp()
fakekubeclient := fakek8s.NewSimpleClientset()
got, err := AddInputResource(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun), logger)
if (err != nil) != c.wantErr {
Expand Down Expand Up @@ -1043,7 +1043,7 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) {
},
}} {
t.Run(c.desc, func(t *testing.T) {
setUp(t)
setUp()
fakekubeclient := fakek8s.NewSimpleClientset(
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand Down
8 changes: 4 additions & 4 deletions pkg/reconciler/taskrun/resources/output_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var (
outputResources map[string]v1alpha1.PipelineResourceInterface
)

func outputResourceSetup(t *testing.T) {
func outputResourceSetup() {
logger, _ = logging.NewLogger("", "")

rs := []*v1alpha1.PipelineResource{{
Expand Down Expand Up @@ -787,7 +787,7 @@ func TestValidOutputResources(t *testing.T) {
}} {
t.Run(c.name, func(t *testing.T) {
names.TestingSeed()
outputResourceSetup(t)
outputResourceSetup()
fakekubeclient := fakek8s.NewSimpleClientset()
got, err := AddOutputResources(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun), logger)
if err != nil {
Expand Down Expand Up @@ -992,7 +992,7 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) {
}}},
}} {
t.Run(c.name, func(t *testing.T) {
outputResourceSetup(t)
outputResourceSetup()
names.TestingSeed()
fakekubeclient := fakek8s.NewSimpleClientset(
&corev1.ConfigMap{
Expand Down Expand Up @@ -1151,7 +1151,7 @@ func TestInvalidOutputResources(t *testing.T) {
wantErr: true,
}} {
t.Run(c.desc, func(t *testing.T) {
outputResourceSetup(t)
outputResourceSetup()
fakekubeclient := fakek8s.NewSimpleClientset()
_, err := AddOutputResources(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun), logger)
if (err != nil) != c.wantErr {
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error

status.SortTaskRunStepOrder(tr.Status.Steps, taskSpec.Steps)

updateTaskRunResourceResult(tr, pod, c.resourceLister, c.KubeClientSet, c.Logger)
updateTaskRunResourceResult(tr, pod, c.resourceLister, c.Logger)

after := tr.Status.GetCondition(apis.ConditionSucceeded)

Expand Down Expand Up @@ -358,7 +358,7 @@ func (c *Reconciler) handlePodCreationError(tr *v1alpha1.TaskRun, err error) {
c.Logger.Errorf("Failed to create build pod for task %q: %v", tr.Name, err)
}

func updateTaskRunResourceResult(taskRun *v1alpha1.TaskRun, pod *corev1.Pod, resourceLister listers.PipelineResourceLister, kubeclient kubernetes.Interface, logger *zap.SugaredLogger) {
func updateTaskRunResourceResult(taskRun *v1alpha1.TaskRun, pod *corev1.Pod, resourceLister listers.PipelineResourceLister, logger *zap.SugaredLogger) {
if resources.TaskRunHasOutputImageResource(resourceLister.PipelineResources(taskRun.Namespace).Get, taskRun) && taskRun.IsSuccessful() {
for _, cs := range pod.Status.ContainerStatuses {
if strings.HasPrefix(cs.Name, imageDigestExporterContainerName) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1359,10 +1359,10 @@ func TestReconcile_SortTaskRunStatusSteps(t *testing.T) {
if err := testAssets.Controller.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil {
t.Errorf("expected no error reconciling valid TaskRun but got %v", err)
}
verify_TaskRunStatusStep(t, taskRun, taskMultipleSteps)
verify_TaskRunStatusStep(t, taskRun)
}

func verify_TaskRunStatusStep(t *testing.T, taskRun *v1alpha1.TaskRun, task *v1alpha1.Task) {
func verify_TaskRunStatusStep(t *testing.T, taskRun *v1alpha1.TaskRun) {
actualStepOrder := []string{}
for _, state := range taskRun.Status.Steps {
actualStepOrder = append(actualStepOrder, state.Name)
Expand Down
4 changes: 2 additions & 2 deletions test/cluster_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestClusterResource(t *testing.T) {
}

t.Logf("Creating Task %s", taskName)
if _, err := c.TaskClient.Create(getClusterResourceTask(namespace, taskName, resourceName, configName)); err != nil {
if _, err := c.TaskClient.Create(getClusterResourceTask(namespace, taskName, configName)); err != nil {
t.Fatalf("Failed to create Task `%s`: %s", taskName, err)
}

Expand Down Expand Up @@ -97,7 +97,7 @@ func getClusterResourceTaskSecret(namespace, name string) *corev1.Secret {
}
}

func getClusterResourceTask(namespace, name, resName, configName string) *v1alpha1.Task {
func getClusterResourceTask(namespace, name, configName string) *v1alpha1.Task {
return tb.Task(name, namespace, tb.TaskSpec(
tb.TaskInputs(tb.InputsResource("target-cluster", v1alpha1.PipelineResourceTypeCluster)),
tb.TaskVolume("config-vol", tb.VolumeSource(corev1.VolumeSource{
Expand Down
6 changes: 3 additions & 3 deletions test/git_checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestGitPipelineRun(t *testing.T) {
}

t.Logf("Creating Task %s", gitTestTaskName)
if _, err := c.TaskClient.Create(getGitCheckTask(namespace, t)); err != nil {
if _, err := c.TaskClient.Create(getGitCheckTask(namespace)); err != nil {
t.Fatalf("Failed to create Task `%s`: %s", gitTestTaskName, err)
}

Expand Down Expand Up @@ -91,7 +91,7 @@ func TestGitPipelineRunFail(t *testing.T) {
}

t.Logf("Creating Task %s", gitTestTaskName)
if _, err := c.TaskClient.Create(getGitCheckTask(namespace, t)); err != nil {
if _, err := c.TaskClient.Create(getGitCheckTask(namespace)); err != nil {
t.Fatalf("Failed to create Task `%s`: %s", gitTestTaskName, err)
}

Expand Down Expand Up @@ -152,7 +152,7 @@ func getGitPipelineResource(namespace, revision string) *v1alpha1.PipelineResour
))
}

func getGitCheckTask(namespace string, t *testing.T) *v1alpha1.Task {
func getGitCheckTask(namespace string) *v1alpha1.Task {
return tb.Task(gitTestTaskName, namespace, tb.TaskSpec(
tb.TaskInputs(tb.InputsResource("gitsource", v1alpha1.PipelineResourceTypeGit)),
tb.Step("git", "alpine/git", tb.StepArgs("--git-dir=/workspace/gitsource/.git", "show")),
Expand Down
14 changes: 7 additions & 7 deletions test/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestMain(m *testing.M) {

func getCRDYaml(cs *clients, ns string) ([]byte, error) {
var output []byte
printOrAdd := func(kind, name string, i interface{}) {
printOrAdd := func(i interface{}) {
bs, err := yaml.Marshal(i)
if err != nil {
return
Expand All @@ -159,46 +159,46 @@ func getCRDYaml(cs *clients, ns string) ([]byte, error) {
return nil, xerrors.Errorf("could not get pipeline: %w", err)
}
for _, i := range ps.Items {
printOrAdd("Pipeline", i.Name, i)
printOrAdd(i)
}

prs, err := cs.PipelineResourceClient.List(metav1.ListOptions{})
if err != nil {
return nil, xerrors.Errorf("could not get pipelinerun resource: %w", err)
}
for _, i := range prs.Items {
printOrAdd("PipelineResource", i.Name, i)
printOrAdd(i)
}

prrs, err := cs.PipelineRunClient.List(metav1.ListOptions{})
if err != nil {
return nil, xerrors.Errorf("could not get pipelinerun: %w", err)
}
for _, i := range prrs.Items {
printOrAdd("PipelineRun", i.Name, i)
printOrAdd(i)
}

ts, err := cs.TaskClient.List(metav1.ListOptions{})
if err != nil {
return nil, xerrors.Errorf("could not get tasks: %w", err)
}
for _, i := range ts.Items {
printOrAdd("Task", i.Name, i)
printOrAdd(i)
}
trs, err := cs.TaskRunClient.List(metav1.ListOptions{})
if err != nil {
return nil, xerrors.Errorf("could not get taskrun: %w", err)
}
for _, i := range trs.Items {
printOrAdd("TaskRun", i.Name, i)
printOrAdd(i)
}

pods, err := cs.KubeClient.Kube.CoreV1().Pods(ns).List(metav1.ListOptions{})
if err != nil {
return nil, xerrors.Errorf("could not get pods: %w", err)
}
for _, i := range pods.Items {
printOrAdd("Pod", i.Name, i)
printOrAdd(i)
}

return output, nil
Expand Down