Skip to content

Commit 5a1acbf

Browse files
authored
enable ginkgolinter and fix findings (operator-framework#6421)
Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
1 parent 25f608b commit 5a1acbf

29 files changed

+165
-192
lines changed

Diff for: .golangci.yml

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ linters:
55
- nakedret
66
- misspell
77
- ineffassign
8+
- ginkgolinter
89
- goconst
910
- goimports
1011
- errcheck

Diff for: internal/ansible/handler/logging_enqueue_annotation_test.go

+4-8
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
9595
}
9696
repl.SetGroupVersionKind(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "ReplicaSet"})
9797

98-
err := handler.SetOwnerAnnotations(podOwner, repl)
99-
Expect(err).To(BeNil())
98+
Expect(handler.SetOwnerAnnotations(podOwner, repl)).To(Succeed())
10099

101100
evt := event.CreateEvent{
102101
Object: repl,
@@ -280,8 +279,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
280279
newPod.Name = pod.Name + "2"
281280
newPod.Namespace = pod.Namespace + "2"
282281

283-
err := handler.SetOwnerAnnotations(podOwner, pod)
284-
Expect(err).To(BeNil())
282+
Expect(handler.SetOwnerAnnotations(podOwner, pod)).To(Succeed())
285283

286284
evt := event.UpdateEvent{
287285
ObjectOld: pod,
@@ -395,8 +393,7 @@ var _ = Describe("LoggingEnqueueRequestForAnnotation", func() {
395393
newPod.Name = pod.Name + "2"
396394
newPod.Namespace = pod.Namespace + "2"
397395

398-
err := handler.SetOwnerAnnotations(podOwner, pod)
399-
Expect(err).To(BeNil())
396+
Expect(handler.SetOwnerAnnotations(podOwner, pod)).To(Succeed())
400397

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

409-
err = handler.SetOwnerAnnotations(podOwner2, newPod)
410-
Expect(err).To(BeNil())
406+
Expect(handler.SetOwnerAnnotations(podOwner2, newPod)).To(Succeed())
411407

412408
evt := event.UpdateEvent{
413409
ObjectOld: pod,

Diff for: internal/ansible/handler/logging_enqueue_object_test.go

+18-23
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
7676
// verify metrics
7777
gauges, err := metrics.Registry.Gather()
7878
Expect(err).NotTo(HaveOccurred())
79-
Expect(len(gauges)).To(Equal(1))
79+
Expect(gauges).To(HaveLen(1))
8080
assertMetrics(gauges[0], 1, []*corev1.Pod{pod})
8181
})
8282
})
@@ -119,7 +119,7 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
119119
// verify metrics
120120
gauges, err := metrics.Registry.Gather()
121121
Expect(err).NotTo(HaveOccurred())
122-
Expect(len(gauges)).To(Equal(0))
122+
Expect(gauges).To(BeEmpty())
123123
})
124124
})
125125
Context("when a gauge does not exist", func() {
@@ -148,7 +148,7 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
148148
// verify metrics
149149
gauges, err := metrics.Registry.Gather()
150150
Expect(err).NotTo(HaveOccurred())
151-
Expect(len(gauges)).To(Equal(0))
151+
Expect(gauges).To(BeEmpty())
152152
})
153153
})
154154

@@ -187,36 +187,31 @@ var _ = Describe("LoggingEnqueueRequestForObject", func() {
187187
// verify metrics
188188
gauges, err := metrics.Registry.Gather()
189189
Expect(err).NotTo(HaveOccurred())
190-
Expect(len(gauges)).To(Equal(1))
190+
Expect(gauges).To(HaveLen(1))
191191
assertMetrics(gauges[0], 2, []*corev1.Pod{newpod, pod})
192192
})
193193
})
194194
})
195195

196196
func assertMetrics(gauge *dto.MetricFamily, count int, pods []*corev1.Pod) {
197-
// need variables to compare the pointers
198-
name := "name"
199-
namespace := "namespace"
200-
g := "group"
201-
v := "version"
202-
k := "kind"
203-
204-
Expect(len(gauge.Metric)).To(Equal(count))
197+
Expect(gauge.Metric).To(HaveLen(count))
205198
for i := 0; i < count; i++ {
206199
Expect(*gauge.Metric[i].Gauge.Value).To(Equal(float64(pods[i].GetObjectMeta().GetCreationTimestamp().UTC().Unix())))
207200

208201
for _, l := range gauge.Metric[i].Label {
209-
switch l.Name {
210-
case &name:
211-
Expect(l.Value).To(Equal(pods[i].GetObjectMeta().GetName()))
212-
case &namespace:
213-
Expect(l.Value).To(Equal(pods[i].GetObjectMeta().GetNamespace()))
214-
case &g:
215-
Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Group))
216-
case &v:
217-
Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Version))
218-
case &k:
219-
Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Kind))
202+
if l.Name != nil {
203+
switch *l.Name {
204+
case "name":
205+
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectMeta().GetName())))
206+
case "namespace":
207+
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectMeta().GetNamespace())))
208+
case "group":
209+
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Group)))
210+
case "version":
211+
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Version)))
212+
case "kind":
213+
Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Kind)))
214+
}
220215
}
221216
}
222217
}

Diff for: internal/ansible/proxy/inject_owner_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ var _ = Describe("injectOwnerReferenceHandler", func() {
112112
}
113113
ownerRefs := modifiedCM.ObjectMeta.OwnerReferences
114114

115-
Expect(len(ownerRefs)).To(Equal(1))
115+
Expect(ownerRefs).To(HaveLen(1))
116116

117117
ownerRef := ownerRefs[0]
118118

Diff for: internal/cmd/ansible-operator/version/cmd_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ var _ = Describe("Running a version command", func() {
4848
w.Close()
4949
}()
5050
stdout, err := io.ReadAll(r)
51-
Expect(err).To(BeNil())
51+
Expect(err).ToNot(HaveOccurred())
5252
stdoutString := string(stdout)
5353
version := ver.GitVersion
5454
if version == "unknown" {

Diff for: internal/cmd/helm-operator/version/cmd_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ var _ = Describe("Running a version command", func() {
4848
w.Close()
4949
}()
5050
stdout, err := io.ReadAll(r)
51-
Expect(err).To(BeNil())
51+
Expect(err).ToNot(HaveOccurred())
5252
stdoutString := string(stdout)
5353
version := ver.GitVersion
5454
if version == "unknown" {

Diff for: internal/cmd/operator-sdk/bundle/cmd_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ var _ = Describe("Running a bundle command", func() {
2626
Expect(cmd).NotTo(BeNil())
2727

2828
subcommands := cmd.Commands()
29-
Expect(len(subcommands)).To(Equal(1))
29+
Expect(subcommands).To(HaveLen(1))
3030
Expect(subcommands[0].Use).To(Equal("validate"))
3131
})
3232
})

Diff for: internal/cmd/operator-sdk/bundle/validate/optional_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ var _ = Describe("Running optional validators", func() {
4646
It("runs no validators for an empty selector", func() {
4747
bundle = &apimanifests.Bundle{}
4848
sel = labels.SelectorFromSet(map[string]string{})
49-
Expect(vals.run(bundle, sel, nil)).To(HaveLen(0))
49+
Expect(vals.run(bundle, sel, nil)).To(BeEmpty())
5050
})
5151
It("runs a validator for one selector on an empty bundle", func() {
5252
bundle = &apimanifests.Bundle{}
@@ -80,22 +80,22 @@ var _ = Describe("Running optional validators", func() {
8080
It("returns an error for an empty selector with no validators", func() {
8181
sel = labels.SelectorFromSet(map[string]string{})
8282
err = vals.checkMatches(sel)
83-
Expect(err).NotTo(BeNil())
83+
Expect(err).To(HaveOccurred())
8484
})
8585
It("returns an error for an unmatched selector with no validators", func() {
8686
sel = labels.SelectorFromSet(map[string]string{
8787
nameKey: "operatorhub",
8888
})
8989
err = vals.checkMatches(sel)
90-
Expect(err).NotTo(BeNil())
90+
Expect(err).To(HaveOccurred())
9191
})
9292
It("returns no error for an unmatched selector with all optional validators", func() {
9393
sel = labels.SelectorFromSet(map[string]string{
9494
nameKey: "operatorhub",
9595
})
9696
vals = optionalValidators
9797
err = vals.checkMatches(sel)
98-
Expect(err).To(BeNil())
98+
Expect(err).ToNot(HaveOccurred())
9999
})
100100
})
101101

Diff for: internal/cmd/operator-sdk/olm/cmd_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ var _ = Describe("Running an olm command", func() {
2828
Expect(cmd.Short).NotTo(BeNil())
2929

3030
subcommands := cmd.Commands()
31-
Expect(len(subcommands)).To(Equal(3))
31+
Expect(subcommands).To(HaveLen(3))
3232
Expect(subcommands[0].Use).To(Equal("install"))
3333
Expect(subcommands[1].Use).To(Equal("status"))
3434
Expect(subcommands[2].Use).To(Equal("uninstall"))

Diff for: internal/cmd/operator-sdk/run/cmd_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ var _ = Describe("Running a run command", func() {
2929
Expect(cmd.Long).NotTo(BeNil())
3030

3131
subcommands := cmd.Commands()
32-
Expect(len(subcommands)).To(Equal(3))
32+
Expect(subcommands).To(HaveLen(3))
3333
Expect(subcommands[0].Use).To(Equal("bundle <bundle-image>"))
3434
Expect(subcommands[1].Use).To(Equal("bundle-upgrade <bundle-image>"))
3535
Expect(subcommands[2].Use).To(Equal("packagemanifests [packagemanifests-root-dir]"))

Diff for: internal/cmd/operator-sdk/run/packagemanifests/packagemanifests_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var _ = Describe("Running a run packagemanifests command", func() {
3131
Expect(cmd.Short).NotTo(BeNil())
3232
Expect(cmd.Long).NotTo(BeNil())
3333
aliases := cmd.Aliases
34-
Expect(len(aliases)).To(Equal(1))
34+
Expect(aliases).To(HaveLen(1))
3535
Expect(aliases[0]).To(Equal("pm"))
3636
})
3737
})

Diff for: internal/generate/clusterserviceversion/bases/definitions/crd_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ var _ = Describe("getTypedDescriptors", func() {
4343

4444
It("handles an empty set of marked fields", func() {
4545
out = getTypedDescriptors(markedFields, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
46-
Expect(out).To(HaveLen(0))
46+
Expect(out).To(BeEmpty())
4747
})
4848
It("returns one spec descriptor for one spec marker on a field", func() {
4949
markedFields[crd.TypeIdent{}] = []*fieldInfo{
@@ -79,7 +79,7 @@ var _ = Describe("getTypedDescriptors", func() {
7979
},
8080
}
8181
out = getTypedDescriptors(markedFields, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
82-
Expect(out).To(HaveLen(0))
82+
Expect(out).To(BeEmpty())
8383
})
8484
It("returns one status descriptor for one status marker on a field", func() {
8585
markedFields[crd.TypeIdent{}] = []*fieldInfo{

Diff for: internal/generate/clusterserviceversion/clusterserviceversion_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ var _ = Describe("Testing CRDs with single version", func() {
259259
}
260260
// Update the input's and expected CSV's Deployment image.
261261
collectManifestsFromFileHelper(g.Collector, goBasicOperatorPath)
262-
Expect(len(g.Collector.Deployments)).To(BeNumerically(">=", 1))
262+
Expect(g.Collector.Deployments).ToNot(BeEmpty())
263263
imageTag := "controller:v" + g.Version
264264
modifyDepImageHelper(&g.Collector.Deployments[0].Spec, imageTag)
265265
updatedCSV := updateCSV(newCSVUIMeta, modifyCSVDepImageHelper(imageTag))
@@ -269,7 +269,7 @@ var _ = Describe("Testing CRDs with single version", func() {
269269
Expect(csv).To(Equal(updatedCSV))
270270

271271
// verify if conversion webhooks are added
272-
Expect(len(csv.Spec.WebhookDefinitions)).NotTo(Equal(0))
272+
Expect(csv.Spec.WebhookDefinitions).NotTo(BeEmpty())
273273
Expect(containsConversionWebhookDefinition(csv.Spec.WebhookDefinitions)).To(BeTrue())
274274
})
275275
})
@@ -424,14 +424,14 @@ func readFileHelper(path string) string {
424424
func modifyCSVDepImageHelper(tag string) func(csv *v1alpha1.ClusterServiceVersion) {
425425
return func(csv *v1alpha1.ClusterServiceVersion) {
426426
depSpecs := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs
427-
ExpectWithOffset(2, len(depSpecs)).To(BeNumerically(">=", 1))
427+
ExpectWithOffset(2, depSpecs).ToNot(BeEmpty())
428428
modifyDepImageHelper(&depSpecs[0].Spec, tag)
429429
}
430430
}
431431

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

Diff for: internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ var _ = Describe("apply functions", func() {
5353

5454
c.Deployments = []appsv1.Deployment{newDeploymentWithLabels(depName, labels)}
5555
applyDeployments(c, strategy)
56-
Expect(len(strategy.DeploymentSpecs)).To(Equal(1))
56+
Expect(strategy.DeploymentSpecs).To(HaveLen(1))
5757
Expect(strategy.DeploymentSpecs[0].Label).To(Equal(labels))
5858
})
5959
})

Diff for: internal/generate/collector/clusterserviceversion_test.go

+18-18
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,16 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
3737
It("returns empty lists for an empty Manifests", func() {
3838
c.Roles = []rbacv1.Role{}
3939
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
40-
Expect(inPerm).To(HaveLen(0))
41-
Expect(inCPerm).To(HaveLen(0))
42-
Expect(out).To(HaveLen(0))
40+
Expect(inPerm).To(BeEmpty())
41+
Expect(inCPerm).To(BeEmpty())
42+
Expect(out).To(BeEmpty())
4343
})
4444

4545
It("splitting 1 Role no RoleBinding", func() {
4646
c.Roles = []rbacv1.Role{newRole("my-role")}
4747
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
48-
Expect(inPerm).To(HaveLen(0))
49-
Expect(inCPerm).To(HaveLen(0))
48+
Expect(inPerm).To(BeEmpty())
49+
Expect(inCPerm).To(BeEmpty())
5050
Expect(out).To(HaveLen(1))
5151
Expect(getRoleNames(out)).To(Equal([]string{"my-role"}))
5252
})
@@ -58,8 +58,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
5858
newRoleBinding("my-role-binding", newRoleRef("my-role"), newServiceAccountSubject("my-other-account")),
5959
}
6060
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
61-
Expect(inPerm).To(HaveLen(0))
62-
Expect(inCPerm).To(HaveLen(0))
61+
Expect(inPerm).To(BeEmpty())
62+
Expect(inCPerm).To(BeEmpty())
6363
Expect(out).To(HaveLen(2))
6464
Expect(getRoleNames(out)).To(Equal([]string{"my-role"}))
6565
Expect(getRoleBindingNames(out)).To(Equal([]string{"my-role-binding"}))
@@ -72,8 +72,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
7272
newRoleBinding("my-role-binding", newClusterRoleRef("my-role"), newServiceAccountSubject("my-other-account")),
7373
}
7474
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
75-
Expect(inPerm).To(HaveLen(0))
76-
Expect(inCPerm).To(HaveLen(0))
75+
Expect(inPerm).To(BeEmpty())
76+
Expect(inCPerm).To(BeEmpty())
7777
Expect(out).To(HaveLen(2))
7878
Expect(getClusterRoleNames(out)).To(Equal([]string{"my-role"}))
7979
Expect(getRoleBindingNames(out)).To(Equal([]string{"my-role-binding"}))
@@ -88,8 +88,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
8888
newRoleBinding("my-role-binding-2", newClusterRoleRef("my-role"), newServiceAccountSubject("my-other-account")),
8989
}
9090
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
91-
Expect(inPerm).To(HaveLen(0))
92-
Expect(inCPerm).To(HaveLen(0))
91+
Expect(inPerm).To(BeEmpty())
92+
Expect(inCPerm).To(BeEmpty())
9393
Expect(out).To(HaveLen(4))
9494
Expect(getRoleNames(out)).To(Equal([]string{"my-role"}))
9595
Expect(getClusterRoleNames(out)).To(Equal([]string{"my-role"}))
@@ -105,8 +105,8 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
105105
inPerm, inCPerm, out = c.SplitCSVPermissionsObjects(nil)
106106
Expect(inPerm).To(HaveLen(1))
107107
Expect(getRoleNames(inPerm)).To(Equal([]string{"my-role"}))
108-
Expect(inCPerm).To(HaveLen(0))
109-
Expect(out).To(HaveLen(0))
108+
Expect(inCPerm).To(BeEmpty())
109+
Expect(out).To(BeEmpty())
110110
})
111111

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

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

141141
It("splitting 1 Role 1 ClusterRole 1 RoleBinding with 2 Subjects containing a Deployment serviceAccountName", func() {
@@ -219,7 +219,7 @@ var _ = Describe("SplitCSVPermissionsObjects", func() {
219219
Expect(getClusterRoleNames(inPerm)).To(Equal([]string{roleName1}))
220220
Expect(inCPerm).To(HaveLen(2))
221221
Expect(getClusterRoleNames(inCPerm)).To(Equal([]string{roleName1, roleName2}))
222-
Expect(out).To(HaveLen(0))
222+
Expect(out).To(BeEmpty())
223223
})
224224
})
225225
})

Diff for: internal/generate/packagemanifest/packagemanifest_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ packageName: memcached-operator
195195
Expect(err).NotTo(HaveOccurred())
196196
Expect(pm).NotTo(BeNil())
197197
Expect(pm.PackageName).To(Equal("memcached-operator"))
198-
Expect(len(pm.Channels)).To(Equal(1))
198+
Expect(pm.Channels).To(HaveLen(1))
199199
Expect(pm.Channels[0].Name).To(Equal("alpha"))
200200
Expect(pm.Channels[0].CurrentCSVName).To(Equal("memcached-operator.v0.0.1"))
201201
Expect(pm.DefaultChannelName).To(Equal("alpha"))

0 commit comments

Comments
 (0)