Merge pull request konflux-ci#485 from MartinBasti/fix-golangci-reports

STONEINTG-661: Fix golangci reported issues

controllers/binding/snapshotenvironmentbinding_adapter_test.go

@@ -399,12 +399,12 @@ var _ = Describe("Binding Adapter", Ordered, func() {
 
 		integrationPipelineRun := integrationPipelineRuns.Items[0]
 		fmt.Fprintf(GinkgoWriter, "*******integrationPipelineRun: %v\n", integrationPipelineRun)
-		Expect(integrationPipelineRun.Labels["appstudio.openshift.io/application"] == hasApp.Name).To(BeTrue())
-		Expect(integrationPipelineRun.Labels["appstudio.openshift.io/component"] == hasComp.Name).To(BeTrue())
-		Expect(integrationPipelineRun.Labels["appstudio.openshift.io/environment"] == hasEnv.Name).To(BeTrue())
-		Expect(integrationPipelineRun.Spec.Workspaces != nil).To(BeTrue())
-		Expect(len(integrationPipelineRun.Spec.Workspaces) > 0).To(BeTrue())
-		Expect(len(integrationPipelineRun.Spec.Params) > 0).To(BeTrue())
+		Expect(integrationPipelineRun.Labels["appstudio.openshift.io/application"]).To(Equal(hasApp.Name))
+		Expect(integrationPipelineRun.Labels["appstudio.openshift.io/component"]).To(Equal(hasComp.Name))
+		Expect(integrationPipelineRun.Labels["appstudio.openshift.io/environment"]).To(Equal(hasEnv.Name))
+		Expect(integrationPipelineRun.Spec.Workspaces).NotTo(BeNil())
+		Expect(integrationPipelineRun.Spec.Workspaces).NotTo(BeEmpty())
+		Expect(integrationPipelineRun.Spec.Params).NotTo(BeEmpty())
 
 		Expect(k8sClient.Delete(ctx, &integrationPipelineRuns.Items[0])).Should(Succeed())
 

controllers/buildpipeline/buildpipeline_adapter_test.go

@@ -19,9 +19,10 @@ package buildpipeline
 import (
 	"bytes"
 	"reflect"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"time"
 
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
 	"github.com/redhat-appstudio/integration-service/gitops"
 	"github.com/redhat-appstudio/integration-service/helpers"
 	"github.com/redhat-appstudio/integration-service/loader"
@@ -689,7 +690,7 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
 			pipelineRuns, err := adapter.getSucceededBuildPipelineRunsForComponent(hasComp)
 			Expect(err).To(BeNil())
 			Expect(pipelineRuns).NotTo(BeNil())
-			Expect(len(*pipelineRuns)).To(Equal(2))
+			Expect(*pipelineRuns).To(HaveLen(2))
 			Expect((*pipelineRuns)[0].Name == buildPipelineRun.Name || (*pipelineRuns)[1].Name == buildPipelineRun.Name).To(BeTrue())
 		})
 

controllers/buildpipeline/buildpipeline_controller.go

@@ -100,8 +100,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, nil
 	}
 
-	application := &applicationapiv1alpha1.Application{}
-	application, err = loader.GetApplicationFromComponent(r.Client, ctx, component)
+	application, err := loader.GetApplicationFromComponent(r.Client, ctx, component)
 	if err != nil {
 		logger.Error(err, "Failed to get Application from Component",
 			"Component.Name ", component.Name, "Component.Namespace ", component.Namespace)

controllers/component/component_adapter_test.go

@@ -128,7 +128,7 @@ var _ = Describe("Component Adapter", Ordered, func() {
 		})
 		snapshots := &applicationapiv1alpha1.SnapshotList{}
 		Eventually(func() bool {
-			k8sClient.List(ctx, snapshots, &client.ListOptions{Namespace: hasApp.Namespace})
+			Expect(k8sClient.List(ctx, snapshots, &client.ListOptions{Namespace: hasApp.Namespace})).To(Succeed())
 			return len(snapshots.Items) == 0
 		}, time.Second*20).Should(BeTrue())
 
@@ -138,7 +138,7 @@ var _ = Describe("Component Adapter", Ordered, func() {
 		result, err := adapter.EnsureComponentIsCleanedUp()
 
 		Eventually(func() bool {
-			k8sClient.List(ctx, snapshots, &client.ListOptions{Namespace: hasApp.Namespace})
+			Expect(k8sClient.List(ctx, snapshots, &client.ListOptions{Namespace: hasApp.Namespace})).To(Succeed())
 			return !result.CancelRequest && len(snapshots.Items) == 1 && err == nil
 		}, time.Second*20).Should(BeTrue())
 	})

controllers/snapshot/snapshot_adapter_test.go

@@ -507,7 +507,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		})
 
 		It("ensures global Component Image will not be updated in the PR context", func() {
-			gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshotPR, "test passed")
+			_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshotPR, "test passed")
+			Expect(err).To(Succeed())
 			Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshotPR)).To(BeTrue())
 			adapter.snapshot = hasSnapshotPR
 
@@ -521,7 +522,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		})
 
 		It("no error from ensuring global Component Image updated when AppStudio Tests failed", func() {
-			gitops.MarkSnapshotAsFailed(k8sClient, ctx, hasSnapshot, "test failed")
+			_, err := gitops.MarkSnapshotAsFailed(k8sClient, ctx, hasSnapshot, "test failed")
+			Expect(err).To(Succeed())
 			Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeFalse())
 			adapter.snapshot = hasSnapshot
 			result, err := adapter.EnsureGlobalCandidateImageUpdated()
@@ -533,7 +535,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		})
 
 		It("ensures global Component Image updated when AppStudio Tests succeeded", func() {
-			gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			Expect(err).To(Succeed())
 			Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
 			adapter.snapshot = hasSnapshot
 
@@ -565,7 +568,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
 
 			gitops.SetSnapshotIntegrationStatusAsFinished(hasSnapshot, "Snapshot integration status condition is finished since all testing pipelines completed")
-			gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			Expect(err).To(Succeed())
 			Expect(gitops.HaveAppStudioTestsFinished(hasSnapshot)).To(BeTrue())
 			Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
 			adapter = NewAdapter(hasSnapshot, hasApp, hasComp, log, loader.NewMockLoader(), k8sClient, ctx)
@@ -639,7 +643,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 					return err
 				}
 				if len(releaseList.Items) > 0 {
-					Expect(releaseList.Items[0].ObjectMeta.Labels["appstudio.openshift.io/component"] == hasComp.Name)
+					Expect(releaseList.Items[0].ObjectMeta.Labels["appstudio.openshift.io/component"]).To(Equal(hasComp.Name))
 				}
 				return nil
 			}, time.Second*10).Should(BeNil())
@@ -704,7 +708,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		It("ensures snapshot environmentBinding exists", func() {
 			log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
 
-			gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			Expect(err).To(Succeed())
 			hasSnapshot.Labels[gitops.PipelineAsCodeEventTypeLabel] = gitops.PipelineAsCodePushType
 			Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
 			adapter = NewAdapter(hasSnapshot, hasApp, hasComp, log, loader.NewMockLoader(), k8sClient, ctx)
@@ -762,7 +767,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			Expect(binding.Spec.Environment).To(Equal(env.Name))
 
 			owners := binding.GetOwnerReferences()
-			Expect(len(owners) == 1).To(BeTrue())
+			Expect(owners).To(HaveLen(1))
 			Expect(owners[0].Name).To(Equal(hasSnapshot.Name))
 
 			// Check if the adapter function detects that it already released the snapshot
@@ -793,7 +798,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 				}
 				Expect(k8sClient.Create(ctx, hasBinding)).Should(Succeed())
 
-				gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+				_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+				Expect(err).To(Succeed())
 				hasSnapshot.Labels[gitops.PipelineAsCodeEventTypeLabel] = gitops.PipelineAsCodePushType
 				Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
 			})
@@ -967,7 +973,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
 
 		BeforeAll(func() {
-			gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+			Expect(err).To(Succeed())
 			Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
 		})
 
@@ -1089,10 +1096,10 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 					Namespace: snapshotEnvironmentBinding.Namespace,
 					Name:      snapshotEnvironmentBinding.Name,
 				}, updatedSEB)
-				return err == nil && len(updatedSEB.Spec.Components) == 1
+				return err == nil && len(updatedSEB.Spec.Components) == 1 && updatedSEB.Spec.Snapshot == otherSnapshot.Name
 			}, time.Second*10).Should(BeTrue())
-			Expect(updatedSEB.Spec.Snapshot == otherSnapshot.Name)
-			Expect(updatedSEB.Spec.Components[0].Name == secondComp.Name)
+			Expect(updatedSEB.Spec.Snapshot).To(Equal(otherSnapshot.Name))
+			Expect(updatedSEB.Spec.Components[0].Name).To(Equal(secondComp.Name))
 
 			err = k8sClient.Delete(ctx, snapshotEnvironmentBinding)
 			Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
@@ -1271,7 +1278,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			Expect(binding.Spec.Environment).To(Equal(ephemeralEnv.Name))
 
 			owners := binding.GetOwnerReferences()
-			Expect(len(owners) == 1).To(BeTrue())
+			Expect(owners).To(HaveLen(1))
 			Expect(owners[0].Name).To(Equal(ephemeralEnv.Name))
 		})
 
@@ -1737,7 +1744,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 				statuses, err := intgteststat.NewSnapshotIntegrationTestStatuses("")
 				Expect(err).To(Succeed())
 				statuses.UpdateTestStatusIfChanged(integrationTestScenarioWithoutEnv.Name, intgteststat.IntegrationTestStatusInProgress, fakeDetails)
-				statuses.UpdateTestPipelineRunName(integrationTestScenarioWithoutEnv.Name, fakePLRName)
+				Expect(statuses.UpdateTestPipelineRunName(integrationTestScenarioWithoutEnv.Name, fakePLRName)).To(Succeed())
 				Expect(gitops.WriteIntegrationTestStatusesIntoSnapshot(hasSnapshot, statuses, k8sClient, ctx)).Should(Succeed())
 
 				// add rerun label

git/github/github_test.go

@@ -281,30 +281,17 @@ var _ = Describe("Client", func() {
 
 		allCheckRuns, err := client.GetAllCheckRunsForRef(context.TODO(), "", "", "", 1)
 		Expect(err).To(BeNil())
-		Expect(len(allCheckRuns) > 0).To(BeTrue())
+		Expect(allCheckRuns).NotTo(BeEmpty())
 
 		existingCheckRun := client.GetExistingCheckRun(allCheckRuns, checkRunAdapter)
 		Expect(existingCheckRun).NotTo(BeNil())
 
-		checkRunAdapter = &github.CheckRunAdapter{
-			Name:           "example-name",
-			Owner:          "example-owner",
-			Repository:     "example-repo",
-			SHA:            "abcdef1",
-			ExternalID:     "example-external-id",
-			Conclusion:     "failure",
-			Title:          "example-title",
-			Summary:        "example-summary",
-			Text:           "example-text-update",
-			StartTime:      time.Now(),
-			CompletionTime: time.Now(),
-		}
 	})
 
 	It("can check if creating a new commit status is needed", func() {
 		commitStatuses, err := client.GetAllCommitStatusesForRef(context.TODO(), "", "", "")
 		Expect(err).To(BeNil())
-		Expect(len(commitStatuses) > 0).To(BeTrue())
+		Expect(commitStatuses).NotTo(BeEmpty())
 
 		commitStatusExist, err := client.CommitStatusExists(commitStatuses, commitStatusAdapter)
 		Expect(commitStatusExist).To(BeTrue())
@@ -326,7 +313,7 @@ var _ = Describe("Client", func() {
 	It("can get existing comment id", func() {
 		comments, err := client.GetAllCommentsForPR(context.TODO(), "", "", 1)
 		Expect(err).To(BeNil())
-		Expect(len(comments) > 0).To(BeTrue())
+		Expect(comments).NotTo(BeEmpty())
 
 		commentID := client.GetExistingCommentID(comments, "snapshotName", "scenarioName")
 		Expect(*commentID).To(Equal(int64(40)))

gitops/binding_test.go

@@ -175,7 +175,8 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() {
 	})
 
 	It("ensures Binding Component is created", func() {
-		gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+		_, err := gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
+		Expect(err).To(Succeed())
 		Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
 		bindingComponents := gitops.NewBindingComponents(hasSnapshot)
 		Expect(bindingComponents).NotTo(BeNil())
@@ -186,7 +187,7 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() {
 		Expect(newSnapshotEnvironmentBinding).NotTo(BeNil())
 		Expect(newSnapshotEnvironmentBinding.Spec.Snapshot).To(Equal(hasSnapshot.Name))
 		Expect(newSnapshotEnvironmentBinding.Spec.Environment).To(Equal(env.Name))
-		Expect(len(newSnapshotEnvironmentBinding.Spec.Components)).To(Equal(1))
+		Expect(newSnapshotEnvironmentBinding.Spec.Components).To(HaveLen(1))
 
 		Expect(gitops.IsBindingDeployed(newSnapshotEnvironmentBinding)).NotTo(BeTrue())
 		Expect(gitops.HaveBindingsFailed(newSnapshotEnvironmentBinding)).NotTo(BeTrue())

gitops/environment_test.go

@@ -384,7 +384,7 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() {
 
 		It("Can return DeploymentTargetClaim object", func() {
 			dtc := gitops.NewDeploymentTargetClaim("default", deploymentTargetClass.Name)
-			Expect(dtc.Spec.DeploymentTargetClassName == applicationapiv1alpha1.DeploymentTargetClassName(deploymentTargetClass.Name)).To(BeTrue())
+			Expect(dtc.Spec.DeploymentTargetClassName).To(Equal(applicationapiv1alpha1.DeploymentTargetClassName(deploymentTargetClass.Name)))
 		})
 	})
 

gitops/snapshot_integration_tests_status_test.go

@@ -81,7 +81,7 @@ var _ = Describe("Snapshot integration test statuses", func() {
 		It("Creates empty statuses when a snaphost doesn't have test status annotation", func() {
 			statuses, err := gitops.NewSnapshotIntegrationTestStatusesFromSnapshot(snapshot)
 			Expect(err).To(BeNil())
-			Expect(len(statuses.GetStatuses())).To(Equal(0))
+			Expect(statuses.GetStatuses()).To(BeEmpty())
 		})
 
 		When("Snapshot contains empty test status annotation", func() {
@@ -94,7 +94,7 @@ var _ = Describe("Snapshot integration test statuses", func() {
 			It("Returns empty test statuses", func() {
 				statuses, err := gitops.NewSnapshotIntegrationTestStatusesFromSnapshot(snapshot)
 				Expect(err).To(BeNil())
-				Expect(len(statuses.GetStatuses())).To(Equal(0))
+				Expect(statuses.GetStatuses()).To(BeEmpty())
 			})
 		})
 
@@ -111,7 +111,7 @@ var _ = Describe("Snapshot integration test statuses", func() {
 			It("Returns expected test statuses", func() {
 				statuses, err := gitops.NewSnapshotIntegrationTestStatusesFromSnapshot(snapshot)
 				Expect(err).To(BeNil())
-				Expect(len(statuses.GetStatuses())).To(Equal(1))
+				Expect(statuses.GetStatuses()).To(HaveLen(1))
 
 				statusDetail := statuses.GetStatuses()[0]
 				Expect(statusDetail.Status).To(Equal(intgteststat.IntegrationTestStatusInProgress))
@@ -191,7 +191,7 @@ var _ = Describe("Snapshot integration test statuses", func() {
 
 				statuses, err := gitops.NewSnapshotIntegrationTestStatusesFromSnapshot(snapshot)
 				Expect(err).To(BeNil())
-				Expect(len(statuses.GetStatuses())).To(Equal(1))
+				Expect(statuses.GetStatuses()).To(HaveLen(1))
 			})
 		})
 

gitops/snapshot_test.go

@@ -135,15 +135,14 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() {
 
 	It("ensures that latest update annotation can be set and get from snapshot", func() {
 		t := time.Time{}
-		t.UnmarshalText([]byte("2023-08-26T17:57:50+02:00"))
+		Expect(t.UnmarshalText([]byte("2023-08-26T17:57:50+02:00"))).To(Succeed())
 		Expect(gitops.GetLatestUpdateTime(hasSnapshot)).To(Equal(t))
 		//set different time
-		t.UnmarshalText([]byte("2023-08-26T18:57:50+02:00"))
-		gitops.SetLatestUpdateTime(hasSnapshot, t)
+		Expect(t.UnmarshalText([]byte("2023-08-26T18:57:50+02:00"))).To(Succeed())
+		Expect(gitops.SetLatestUpdateTime(hasSnapshot, t)).To(Succeed())
 		Expect(hasSnapshot.GetAnnotations()[gitops.SnapshotPRLastUpdate]).To(Equal("2023-08-26T18:57:50+02:00"))
-		//set latest update time to nil
-		t.UnmarshalText([]byte(""))
-		gitops.SetLatestUpdateTime(hasSnapshot, t)
+		//set latest update time to zero
+		Expect(gitops.SetLatestUpdateTime(hasSnapshot, time.Time{})).To(Succeed())
 		Expect(hasSnapshot.GetAnnotations()[gitops.SnapshotPRLastUpdate]).To(Equal("0001-01-01T00:00:00Z"))
 
 	})