Skip to content

Commit

Permalink
enable ginkgolinter and fix findings (#6421)
Browse files Browse the repository at this point in the history
Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
  • Loading branch information
nunnatsa committed May 18, 2023
1 parent 25f608b commit 5a1acbf
Show file tree
Hide file tree
Showing 29 changed files with 165 additions and 192 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ linters:
- nakedret
- misspell
- ineffassign
- ginkgolinter
- goconst
- goimports
- errcheck
Expand Down
12 changes: 4 additions & 8 deletions internal/ansible/handler/logging_enqueue_annotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
}
repl.SetGroupVersionKind(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "ReplicaSet"})

err := handler.SetOwnerAnnotations(podOwner, repl)
Expect(err).To(BeNil())
Expect(handler.SetOwnerAnnotations(podOwner, repl)).To(Succeed())

evt := event.CreateEvent{
Object: repl,
Expand Down Expand Up @@ -280,8 +279,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
newPod.Name = pod.Name + "2"
newPod.Namespace = pod.Namespace + "2"

err := handler.SetOwnerAnnotations(podOwner, pod)
Expect(err).To(BeNil())
Expect(handler.SetOwnerAnnotations(podOwner, pod)).To(Succeed())

evt := event.UpdateEvent{
ObjectOld: pod,
Expand Down Expand Up @@ -395,8 +393,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
newPod.Name = pod.Name + "2"
newPod.Namespace = pod.Namespace + "2"

err := handler.SetOwnerAnnotations(podOwner, pod)
Expect(err).To(BeNil())
Expect(handler.SetOwnerAnnotations(podOwner, pod)).To(Succeed())

var podOwner2 = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -406,8 +403,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
}
podOwner2.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Kind: "Pod"})

err = handler.SetOwnerAnnotations(podOwner2, newPod)
Expect(err).To(BeNil())
Expect(handler.SetOwnerAnnotations(podOwner2, newPod)).To(Succeed())

evt := event.UpdateEvent{
ObjectOld: pod,
Expand Down
41 changes: 18 additions & 23 deletions internal/ansible/handler/logging_enqueue_object_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
// verify metrics
gauges, err := metrics.Registry.Gather()
Expect(err).NotTo(HaveOccurred())
Expect(len(gauges)).To(Equal(1))
Expect(gauges).To(HaveLen(1))
assertMetrics(gauges[0], 1, []*corev1.Pod{pod})
})
})
Expand Down Expand Up @@ -119,7 +119,7 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
// verify metrics
gauges, err := metrics.Registry.Gather()
Expect(err).NotTo(HaveOccurred())
Expect(len(gauges)).To(Equal(0))
Expect(gauges).To(BeEmpty())
})
})
Context("when a gauge does not exist", func() {
Expand Down Expand Up @@ -148,7 +148,7 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
// verify metrics
gauges, err := metrics.Registry.Gather()
Expect(err).NotTo(HaveOccurred())
Expect(len(gauges)).To(Equal(0))
Expect(gauges).To(BeEmpty())
})
})

Expand Down Expand Up @@ -187,36 +187,31 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
// verify metrics
gauges, err := metrics.Registry.Gather()
Expect(err).NotTo(HaveOccurred())
Expect(len(gauges)).To(Equal(1))
Expect(gauges).To(HaveLen(1))
assertMetrics(gauges[0], 2, []*corev1.Pod{newpod, pod})
})
})
})

func assertMetrics(gauge *dto.MetricFamily, count int, pods []*corev1.Pod) {
// need variables to compare the pointers
name := "name"
namespace := "namespace"
g := "group"
v := "version"
k := "kind"

Expect(len(gauge.Metric)).To(Equal(count))
Expect(gauge.Metric).To(HaveLen(count))
for i := 0; i < count; i++ {
Expect(*gauge.Metric[i].Gauge.Value).To(Equal(float64(pods[i].GetObjectMeta().GetCreationTimestamp().UTC().Unix())))

for _, l := range gauge.Metric[i].Label {
switch l.Name {
case &name:
Expect(l.Value).To(Equal(pods[i].GetObjectMeta().GetName()))
case &namespace:
Expect(l.Value).To(Equal(pods[i].GetObjectMeta().GetNamespace()))
case &g:
Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Group))
case &v:
Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Version))
case &k:
Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Kind))
if l.Name != nil {
switch *l.Name {
case "name":
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectMeta().GetName())))
case "namespace":
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectMeta().GetNamespace())))
case "group":
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Group)))
case "version":
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Version)))
case "kind":
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Kind)))
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/ansible/proxy/inject_owner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ var _ = Describe("injectOwnerReferenceHandler", func() {
}
ownerRefs := modifiedCM.ObjectMeta.OwnerReferences

Expect(len(ownerRefs)).To(Equal(1))
Expect(ownerRefs).To(HaveLen(1))

ownerRef := ownerRefs[0]

Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/ansible-operator/version/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var _ = Describe("Running a version command", func() {
w.Close()
}()
stdout, err := io.ReadAll(r)
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
stdoutString := string(stdout)
version := ver.GitVersion
if version == "unknown" {
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/helm-operator/version/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var _ = Describe("Running a version command", func() {
w.Close()
}()
stdout, err := io.ReadAll(r)
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
stdoutString := string(stdout)
version := ver.GitVersion
if version == "unknown" {
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/operator-sdk/bundle/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var _ = Describe("Running a bundle command", func() {
Expect(cmd).NotTo(BeNil())

subcommands := cmd.Commands()
Expect(len(subcommands)).To(Equal(1))
Expect(subcommands).To(HaveLen(1))
Expect(subcommands[0].Use).To(Equal("validate"))
})
})
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/operator-sdk/bundle/validate/optional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("Running optional validators", func() {
It("runs no validators for an empty selector", func() {
bundle = &apimanifests.Bundle{}
sel = labels.SelectorFromSet(map[string]string{})
Expect(vals.run(bundle, sel, nil)).To(HaveLen(0))
Expect(vals.run(bundle, sel, nil)).To(BeEmpty())
})
It("runs a validator for one selector on an empty bundle", func() {
bundle = &apimanifests.Bundle{}
Expand Down Expand Up @@ -80,22 +80,22 @@ var _ = Describe("Running optional validators", func() {
It("returns an error for an empty selector with no validators", func() {
sel = labels.SelectorFromSet(map[string]string{})
err = vals.checkMatches(sel)
Expect(err).NotTo(BeNil())
Expect(err).To(HaveOccurred())
})
It("returns an error for an unmatched selector with no validators", func() {
sel = labels.SelectorFromSet(map[string]string{
nameKey: "operatorhub",
})
err = vals.checkMatches(sel)
Expect(err).NotTo(BeNil())
Expect(err).To(HaveOccurred())
})
It("returns no error for an unmatched selector with all optional validators", func() {
sel = labels.SelectorFromSet(map[string]string{
nameKey: "operatorhub",
})
vals = optionalValidators
err = vals.checkMatches(sel)
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
})
})

Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/operator-sdk/olm/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var _ = Describe("Running an olm command", func() {
Expect(cmd.Short).NotTo(BeNil())

subcommands := cmd.Commands()
Expect(len(subcommands)).To(Equal(3))
Expect(subcommands).To(HaveLen(3))
Expect(subcommands[0].Use).To(Equal("install"))
Expect(subcommands[1].Use).To(Equal("status"))
Expect(subcommands[2].Use).To(Equal("uninstall"))
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/operator-sdk/run/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ var _ = Describe("Running a run command", func() {
Expect(cmd.Long).NotTo(BeNil())

subcommands := cmd.Commands()
Expect(len(subcommands)).To(Equal(3))
Expect(subcommands).To(HaveLen(3))
Expect(subcommands[0].Use).To(Equal("bundle <bundle-image>"))
Expect(subcommands[1].Use).To(Equal("bundle-upgrade <bundle-image>"))
Expect(subcommands[2].Use).To(Equal("packagemanifests [packagemanifests-root-dir]"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var _ = Describe("Running a run packagemanifests command", func() {
Expect(cmd.Short).NotTo(BeNil())
Expect(cmd.Long).NotTo(BeNil())
aliases := cmd.Aliases
Expect(len(aliases)).To(Equal(1))
Expect(aliases).To(HaveLen(1))
Expect(aliases[0]).To(Equal("pm"))
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var _ = Describe("getTypedDescriptors", func() {

It("handles an empty set of marked fields", func() {
out = getTypedDescriptors(markedFields, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
Expect(out).To(HaveLen(0))
Expect(out).To(BeEmpty())
})
It("returns one spec descriptor for one spec marker on a field", func() {
markedFields[crd.TypeIdent{}] = []*fieldInfo{
Expand Down Expand Up @@ -79,7 +79,7 @@ var _ = Describe("getTypedDescriptors", func() {
},
}
out = getTypedDescriptors(markedFields, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
Expect(out).To(HaveLen(0))
Expect(out).To(BeEmpty())
})
It("returns one status descriptor for one status marker on a field", func() {
markedFields[crd.TypeIdent{}] = []*fieldInfo{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ var _ = Describe("Testing CRDs with single version", func() {
}
// Update the input's and expected CSV's Deployment image.
collectManifestsFromFileHelper(g.Collector, goBasicOperatorPath)
Expect(len(g.Collector.Deployments)).To(BeNumerically(">=", 1))
Expect(g.Collector.Deployments).ToNot(BeEmpty())
imageTag := "controller:v" + g.Version
modifyDepImageHelper(&g.Collector.Deployments[0].Spec, imageTag)
updatedCSV := updateCSV(newCSVUIMeta, modifyCSVDepImageHelper(imageTag))
Expand All @@ -269,7 +269,7 @@ var _ = Describe("Testing CRDs with single version", func() {
Expect(csv).To(Equal(updatedCSV))

// verify if conversion webhooks are added
Expect(len(csv.Spec.WebhookDefinitions)).NotTo(Equal(0))
Expect(csv.Spec.WebhookDefinitions).NotTo(BeEmpty())
Expect(containsConversionWebhookDefinition(csv.Spec.WebhookDefinitions)).To(BeTrue())
})
})
Expand Down Expand Up @@ -424,14 +424,14 @@ func readFileHelper(path string) string {
func modifyCSVDepImageHelper(tag string) func(csv *v1alpha1.ClusterServiceVersion) {
return func(csv *v1alpha1.ClusterServiceVersion) {
depSpecs := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs
ExpectWithOffset(2, len(depSpecs)).To(BeNumerically(">=", 1))
ExpectWithOffset(2, depSpecs).ToNot(BeEmpty())
modifyDepImageHelper(&depSpecs[0].Spec, tag)
}
}

func modifyDepImageHelper(depSpec *appsv1.DeploymentSpec, tag string) {
containers := depSpec.Template.Spec.Containers
ExpectWithOffset(1, len(containers)).To(BeNumerically(">=", 1))
ExpectWithOffset(1, containers).ToNot(BeEmpty())
containers[0].Image = tag
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var _ = Describe("apply functions", func() {

c.Deployments = []appsv1.Deployment{newDeploymentWithLabels(depName, labels)}
applyDeployments(c, strategy)
Expect(len(strategy.DeploymentSpecs)).To(Equal(1))
Expect(strategy.DeploymentSpecs).To(HaveLen(1))
Expect(strategy.DeploymentSpecs[0].Label).To(Equal(labels))
})
})
Expand Down
36 changes: 18 additions & 18 deletions internal/generate/collector/clusterserviceversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
It("returns empty lists for an empty Manifests", func() {
c.Roles = []rbacv1.Role{}
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(0))
Expect(inCPerm).To(HaveLen(0))
Expect(out).To(HaveLen(0))
Expect(inPerm).To(BeEmpty())
Expect(inCPerm).To(BeEmpty())
Expect(out).To(BeEmpty())
})

It("splitting 1 Role no RoleBinding", func() {
c.Roles = []rbacv1.Role{newRole("my-role")}
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(0))
Expect(inCPerm).To(HaveLen(0))
Expect(inPerm).To(BeEmpty())
Expect(inCPerm).To(BeEmpty())
Expect(out).To(HaveLen(1))
Expect(getRoleNames(out)).To(Equal([]string{"my-role"}))
})
Expand All @@ -58,8 +58,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
newRoleBinding("my-role-binding", newRoleRef("my-role"), newServiceAccountSubject("my-other-account")),
}
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(0))
Expect(inCPerm).To(HaveLen(0))
Expect(inPerm).To(BeEmpty())
Expect(inCPerm).To(BeEmpty())
Expect(out).To(HaveLen(2))
Expect(getRoleNames(out)).To(Equal([]string{"my-role"}))
Expect(getRoleBindingNames(out)).To(Equal([]string{"my-role-binding"}))
Expand All @@ -72,8 +72,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
newRoleBinding("my-role-binding", newClusterRoleRef("my-role"), newServiceAccountSubject("my-other-account")),
}
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(0))
Expect(inCPerm).To(HaveLen(0))
Expect(inPerm).To(BeEmpty())
Expect(inCPerm).To(BeEmpty())
Expect(out).To(HaveLen(2))
Expect(getClusterRoleNames(out)).To(Equal([]string{"my-role"}))
Expect(getRoleBindingNames(out)).To(Equal([]string{"my-role-binding"}))
Expand All @@ -88,8 +88,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
newRoleBinding("my-role-binding-2", newClusterRoleRef("my-role"), newServiceAccountSubject("my-other-account")),
}
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(0))
Expect(inCPerm).To(HaveLen(0))
Expect(inPerm).To(BeEmpty())
Expect(inCPerm).To(BeEmpty())
Expect(out).To(HaveLen(4))
Expect(getRoleNames(out)).To(Equal([]string{"my-role"}))
Expect(getClusterRoleNames(out)).To(Equal([]string{"my-role"}))
Expand All @@ -105,8 +105,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(1))
Expect(getRoleNames(inPerm)).To(Equal([]string{"my-role"}))
Expect(inCPerm).To(HaveLen(0))
Expect(out).To(HaveLen(0))
Expect(inCPerm).To(BeEmpty())
Expect(out).To(BeEmpty())
})

It("splitting 1 ClusterRole 1 RoleBinding with 1 Subject containing a Deployment serviceAccountName", func() {
Expand All @@ -118,8 +118,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
Expect(inPerm).To(HaveLen(1))
Expect(getClusterRoleNames(inPerm)).To(Equal([]string{"my-role"}))
Expect(inCPerm).To(HaveLen(0))
Expect(out).To(HaveLen(0))
Expect(inCPerm).To(BeEmpty())
Expect(out).To(BeEmpty())
})

It("splitting 1 Role 1 ClusterRole 1 RoleBinding with 1 Subject containing a Deployment serviceAccountName", func() {
Expand All @@ -134,8 +134,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
Expect(inPerm).To(HaveLen(2))
Expect(getRoleNames(inPerm)).To(Equal([]string{"my-role"}))
Expect(getClusterRoleNames(inPerm)).To(Equal([]string{"my-role"}))
Expect(inCPerm).To(HaveLen(0))
Expect(out).To(HaveLen(0))
Expect(inCPerm).To(BeEmpty())
Expect(out).To(BeEmpty())
})

It("splitting 1 Role 1 ClusterRole 1 RoleBinding with 2 Subjects containing a Deployment serviceAccountName", func() {
Expand Down Expand Up @@ -219,7 +219,7 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
Expect(getClusterRoleNames(inPerm)).To(Equal([]string{roleName1}))
Expect(inCPerm).To(HaveLen(2))
Expect(getClusterRoleNames(inCPerm)).To(Equal([]string{roleName1, roleName2}))
Expect(out).To(HaveLen(0))
Expect(out).To(BeEmpty())
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion internal/generate/packagemanifest/packagemanifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ packageName: memcached-operator
Expect(err).NotTo(HaveOccurred())
Expect(pm).NotTo(BeNil())
Expect(pm.PackageName).To(Equal("memcached-operator"))
Expect(len(pm.Channels)).To(Equal(1))
Expect(pm.Channels).To(HaveLen(1))
Expect(pm.Channels[0].Name).To(Equal("alpha"))
Expect(pm.Channels[0].CurrentCSVName).To(Equal("memcached-operator.v0.0.1"))
Expect(pm.DefaultChannelName).To(Equal("alpha"))
Expand Down
Loading

0 comments on commit 5a1acbf

Please sign in to comment.