diff --git a/.golangci.yml b/.golangci.yml index cda2f0c..749e483 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ linters: - asciicheck - bodyclose - errorlint + - ginkgolinter - gofmt - goimports - gosec diff --git a/conditions/conditions_test.go b/conditions/conditions_test.go index a9001f9..581b626 100644 --- a/conditions/conditions_test.go +++ b/conditions/conditions_test.go @@ -64,25 +64,21 @@ var _ = Describe("Condition", func() { } // Create Operator Condition - err := os.Setenv(operatorCondEnvVar, "operator-condition-test") - Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(operatorCondEnvVar, "operator-condition-test")).To(Succeed()) readNamespace = func() (string, error) { return ns, nil } // create a new client sch := runtime.NewScheme() - err = apiv2.AddToScheme(sch) - Expect(err).NotTo(HaveOccurred()) + Expect(apiv2.AddToScheme(sch)).To(Succeed()) cl = fake.NewClientBuilder().WithScheme(sch).WithStatusSubresource(operatorCond).Build() // create an operator Condition resource - err = cl.Create(ctx, operatorCond) - Expect(err).NotTo(HaveOccurred()) + Expect(cl.Create(ctx, operatorCond)).To(Succeed()) // Update its status - err = cl.Status().Update(ctx, operatorCond) - Expect(err).NotTo(HaveOccurred()) + Expect(cl.Status().Update(ctx, operatorCond)).To(Succeed()) }) AfterEach(func() { @@ -107,14 +103,12 @@ var _ = Describe("Condition", func() { Expect(err).NotTo(HaveOccurred()) con, err := c.Get(ctx) - Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("conditionType %v not found", conditionBar)))) Expect(con).To(BeNil()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("conditionType %v not found", conditionBar))) }) It("should error when operator Condition is not present in cluster", func() { - err := os.Setenv(operatorCondEnvVar, "NON_EXISTING_COND") - Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(operatorCondEnvVar, "NON_EXISTING_COND")).To(Succeed()) By("setting the status of a new condition") c, err := NewCondition(cl, conditionFoo) @@ -131,13 +125,11 @@ var _ = Describe("Condition", func() { By("setting the status of an existing condition") c, err := NewCondition(cl, conditionFoo) Expect(err).NotTo(HaveOccurred()) - err = c.Set(ctx, metav1.ConditionFalse, WithReason("not_in_foo_state"), WithMessage("test")) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Set(ctx, metav1.ConditionFalse, WithReason("not_in_foo_state"), WithMessage("test"))).To(Succeed()) By("fetching the condition from cluster") op := &apiv2.OperatorCondition{} - err = cl.Get(ctx, objKey, op) - Expect(err).NotTo(HaveOccurred()) + Expect(cl.Get(ctx, objKey, op)).To(Succeed()) By("checking if the condition has been updated") res := op.Spec.Conditions[0] @@ -150,22 +142,19 @@ var _ = Describe("Condition", func() { By("setting the status of a new condition") c, err := NewCondition(cl, conditionBar) Expect(err).NotTo(HaveOccurred()) - err = c.Set(ctx, metav1.ConditionTrue, WithReason("in_bar_state"), WithMessage("test")) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Set(ctx, metav1.ConditionTrue, WithReason("in_bar_state"), WithMessage("test"))).To(Succeed()) By("fetching the condition from cluster") op := &apiv2.OperatorCondition{} - err = cl.Get(ctx, objKey, op) - Expect(err).NotTo(HaveOccurred()) + Expect(cl.Get(ctx, objKey, op)).To(Succeed()) By("checking if the condition has been updated") res := op.Spec.Conditions - Expect(len(res)).To(BeEquivalentTo(2)) + Expect(res).To(HaveLen(2)) Expect(meta.IsStatusConditionTrue(res, string(conditionBar))).To(BeTrue()) }) It("should error when operator Condition is not present in cluster", func() { - err := os.Setenv(operatorCondEnvVar, "NON_EXISTING_COND") - Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(operatorCondEnvVar, "NON_EXISTING_COND")).To(Succeed()) By("setting the status of a new condition") c, err := NewCondition(cl, conditionBar) diff --git a/conditions/factory_test.go b/conditions/factory_test.go index fdb339f..5973a3a 100644 --- a/conditions/factory_test.go +++ b/conditions/factory_test.go @@ -36,8 +36,7 @@ var _ = Describe("NewCondition", func() { var cl client.Client BeforeEach(func() { sch := runtime.NewScheme() - err := apiv2.AddToScheme(sch) - Expect(err).NotTo(HaveOccurred()) + Expect(apiv2.AddToScheme(sch)).To(Succeed()) cl = fake.NewClientBuilder().WithScheme(sch).Build() }) @@ -56,8 +55,7 @@ var _ = Describe("InClusterFactory", func() { BeforeEach(func() { sch := runtime.NewScheme() - err := apiv2.AddToScheme(sch) - Expect(err).NotTo(HaveOccurred()) + Expect(apiv2.AddToScheme(sch)).To(Succeed()) cl = fake.NewClientBuilder().WithScheme(sch).Build() f = InClusterFactory{cl} }) @@ -73,8 +71,7 @@ var _ = Describe("InClusterFactory", func() { func testNewCondition(fn func(apiv2.ConditionType) (Condition, error)) { It("should create a new condition", func() { - err := os.Setenv(operatorCondEnvVar, "test-operator-condition") - Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(operatorCondEnvVar, "test-operator-condition")).To(Succeed()) readNamespace = func() (string, error) { return "default", nil } @@ -85,8 +82,7 @@ func testNewCondition(fn func(apiv2.ConditionType) (Condition, error)) { }) It("should error when namespacedName cannot be found", func() { - err := os.Unsetenv(operatorCondEnvVar) - Expect(err).NotTo(HaveOccurred()) + Expect(os.Unsetenv(operatorCondEnvVar)).To(Succeed()) c, err := fn(conditionFoo) Expect(err).To(HaveOccurred()) @@ -96,32 +92,27 @@ func testNewCondition(fn func(apiv2.ConditionType) (Condition, error)) { func testGetNamespacedName(fn func() (*types.NamespacedName, error)) { It("should error when name of the operator condition cannot be found", func() { - err := os.Unsetenv(operatorCondEnvVar) - Expect(err).NotTo(HaveOccurred()) + Expect(os.Unsetenv(operatorCondEnvVar)).To(Succeed()) objKey, err := fn() - Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("could not determine operator condition name"))) Expect(objKey).To(BeNil()) - Expect(err.Error()).To(ContainSubstring("could not determine operator condition name")) }) It("should error when object namespace cannot be found", func() { - err := os.Setenv(operatorCondEnvVar, "test") - Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(operatorCondEnvVar, "test")).To(Succeed()) readNamespace = func() (string, error) { return "", os.ErrNotExist } objKey, err := fn() - Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("get operator condition namespace: file does not exist"))) Expect(objKey).To(BeNil()) - Expect(err.Error()).To(ContainSubstring("get operator condition namespace: file does not exist")) }) It("should return the right namespaced name from SA namespace file", func() { - err := os.Setenv(operatorCondEnvVar, "test") - Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(operatorCondEnvVar, "test")).To(Succeed()) readNamespace = func() (string, error) { return "testns", nil @@ -135,6 +126,5 @@ func testGetNamespacedName(fn func() (*types.NamespacedName, error)) { } func deleteCondition(ctx context.Context, client client.Client, obj client.Object) { - err := client.Delete(ctx, obj) - Expect(err).NotTo(HaveOccurred()) + Expect(client.Delete(ctx, obj)).To(Succeed()) } diff --git a/handler/enqueue_annotation_test.go b/handler/enqueue_annotation_test.go index ba32bda..8835aa9 100644 --- a/handler/enqueue_annotation_test.go +++ b/handler/enqueue_annotation_test.go @@ -56,8 +56,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { podOwner.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Kind: "Pod"}) - err := SetOwnerAnnotations(podOwner, pod) - Expect(err).To(BeNil()) + Expect(SetOwnerAnnotations(podOwner, pod)).To(Succeed()) instance = EnqueueRequestForAnnotation{ Type: schema.GroupKind{ Group: "", @@ -92,8 +91,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { }, } - err := SetOwnerAnnotations(podOwner, repl) - Expect(err).To(BeNil()) + Expect(SetOwnerAnnotations(podOwner, repl)).To(Succeed()) evt := event.CreateEvent{ Object: repl, @@ -248,8 +246,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { newPod.Name = pod.Name + "2" newPod.Namespace = pod.Namespace + "2" - err := SetOwnerAnnotations(podOwner, pod) - Expect(err).To(BeNil()) + Expect(SetOwnerAnnotations(podOwner, pod)).To(Succeed()) evt := event.UpdateEvent{ ObjectOld: pod, @@ -337,8 +334,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { newPod.Name = pod.Name + "2" newPod.Namespace = pod.Namespace + "2" - err := SetOwnerAnnotations(podOwner, pod) - Expect(err).To(BeNil()) + Expect(SetOwnerAnnotations(podOwner, pod)).To(Succeed()) var podOwner2 = &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -348,8 +344,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { } podOwner2.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Kind: "Pod"}) - err = SetOwnerAnnotations(podOwner2, newPod) - Expect(err).To(BeNil()) + Expect(SetOwnerAnnotations(podOwner2, newPod)).To(Succeed()) evt := event.UpdateEvent{ ObjectOld: pod, @@ -389,8 +384,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { }, } - err := SetOwnerAnnotations(podOwner, nd) - Expect(err).To(BeNil()) + Expect(SetOwnerAnnotations(podOwner, nd)).To(Succeed()) expected := map[string]string{ "my-test-annotation": "should-keep", @@ -398,7 +392,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { TypeAnnotation: schema.GroupKind{Group: "", Kind: "Pod"}.String(), } - Expect(len(nd.GetAnnotations())).To(Equal(3)) + Expect(nd.GetAnnotations()).To(HaveLen(3)) Expect(nd.GetAnnotations()).To(Equal(expected)) }) It("should return error when the owner Kind is not present", func() { @@ -409,8 +403,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { } podOwner.SetGroupVersionKind(schema.GroupVersionKind{Group: "Pod", Kind: ""}) - err := SetOwnerAnnotations(podOwner, nd) - Expect(err).NotTo(BeNil()) + Expect(SetOwnerAnnotations(podOwner, nd)).ToNot(Succeed()) }) It("should return an error when the owner Name is not set", func() { nd := &corev1.Node{ @@ -426,8 +419,7 @@ var _ = Describe("EnqueueRequestForAnnotation", func() { } ownerNew.SetGroupVersionKind(schema.GroupVersionKind{Group: "Pod", Kind: ""}) - err := SetOwnerAnnotations(ownerNew, nd) - Expect(err).NotTo(BeNil()) + Expect(SetOwnerAnnotations(ownerNew, nd)).ToNot(Succeed()) }) }) }) diff --git a/handler/instrumented_enqueue_object_test.go b/handler/instrumented_enqueue_object_test.go index 60911f4..946854b 100644 --- a/handler/instrumented_enqueue_object_test.go +++ b/handler/instrumented_enqueue_object_test.go @@ -79,7 +79,7 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { // verify metrics gauges, err := registry.Gather() Expect(err).NotTo(HaveOccurred()) - Expect(len(gauges)).To(Equal(1)) + Expect(gauges).To(HaveLen(1)) assertMetrics(gauges[0], 1, []*corev1.Pod{pod}) }) }) @@ -114,7 +114,7 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { // verify metrics gauges, err := registry.Gather() Expect(err).NotTo(HaveOccurred()) - Expect(len(gauges)).To(Equal(0)) + Expect(gauges).To(BeEmpty()) }) }) Context("when a gauge does not exist", func() { @@ -139,7 +139,7 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { // verify metrics gauges, err := registry.Gather() Expect(err).NotTo(HaveOccurred()) - Expect(len(gauges)).To(Equal(0)) + Expect(gauges).To(BeEmpty()) }) }) @@ -174,7 +174,7 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { // verify metrics gauges, err := 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}) }) }) @@ -183,7 +183,7 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { It("should fill out map with values from given objects", func() { labelMap := getResourceLabels(pod) Expect(labelMap).ShouldNot(BeEmpty()) - Expect(len(labelMap)).To(Equal(5)) + Expect(labelMap).To(HaveLen(5)) Expect(labelMap["name"]).To(Equal(pod.GetObjectMeta().GetName())) Expect(labelMap["namespace"]).To(Equal(pod.GetObjectMeta().GetNamespace())) Expect(labelMap["group"]).To(Equal(pod.GetObjectKind().GroupVersionKind().Group)) @@ -195,28 +195,30 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { 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)) + const ( + name = "name" + namespace = "namespace" + g = "group" + v = "version" + k = "kind" + ) + + 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)) + 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 g: + Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Group))) + case v: + Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Version))) + case k: + Expect(l.Value).To(HaveValue(Equal(pods[i].GetObjectKind().GroupVersionKind().Kind))) } } } diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 1c7e11c..b3bb7a1 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -38,7 +38,7 @@ var _ = Describe("Helpers test", func() { // test namespace, err := GetOperatorNamespace() - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(namespace).To(Equal("testnamespace")) }) It("should trim whitespace from namespace", func() { @@ -48,7 +48,7 @@ var _ = Describe("Helpers test", func() { // test namespace, err := GetOperatorNamespace() - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(namespace).To(Equal("testnamespace")) }) }) diff --git a/leader/leader_test.go b/leader/leader_test.go index b1a1c2e..4798795 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -16,7 +16,6 @@ package leader import ( "context" - "errors" "os" . "github.com/onsi/ginkgo/v2" @@ -65,8 +64,7 @@ var _ = Describe("Leader election", func() { }) It("should return an error when POD_NAME is not set", func() { os.Unsetenv("POD_NAME") - err := Become(context.TODO(), "leader-test") - Expect(err).ShouldNot(BeNil()) + Expect(Become(context.TODO(), "leader-test")).ShouldNot(Succeed()) }) It("should return an ErrNoNamespace", func() { os.Setenv("POD_NAME", "leader-test") @@ -74,9 +72,7 @@ var _ = Describe("Leader election", func() { return "", ErrNoNamespace } err := Become(context.TODO(), "leader-test", WithClient(client)) - Expect(err).ShouldNot(BeNil()) - Expect(err).To(Equal(ErrNoNamespace)) - Expect(errors.Is(err, ErrNoNamespace)).To(Equal(true)) + Expect(err).Should(MatchError(ErrNoNamespace)) }) It("should not return an error", func() { os.Setenv("POD_NAME", "leader-test") @@ -84,8 +80,7 @@ var _ = Describe("Leader election", func() { return "testns", nil } - err := Become(context.TODO(), "leader-test", WithClient(client)) - Expect(err).Should(BeNil()) + Expect(Become(context.TODO(), "leader-test", WithClient(client))).To(Succeed()) }) }) Describe("isPodEvicted", func() { @@ -96,21 +91,21 @@ var _ = Describe("Leader election", func() { leaderPod = &corev1.Pod{} }) It("should return false with an empty status", func() { - Expect(isPodEvicted(*leaderPod)).To(Equal(false)) + Expect(isPodEvicted(*leaderPod)).To(BeFalse()) }) It("should return false if reason is incorrect", func() { leaderPod.Status.Phase = corev1.PodFailed leaderPod.Status.Reason = "invalid" - Expect(isPodEvicted(*leaderPod)).To(Equal(false)) + Expect(isPodEvicted(*leaderPod)).To(BeFalse()) }) It("should return false if pod is in the wrong phase", func() { leaderPod.Status.Phase = corev1.PodRunning - Expect(isPodEvicted(*leaderPod)).To(Equal(false)) + Expect(isPodEvicted(*leaderPod)).To(BeFalse()) }) It("should return true when Phase and Reason are set", func() { leaderPod.Status.Phase = corev1.PodFailed leaderPod.Status.Reason = "Evicted" - Expect(isPodEvicted(*leaderPod)).To(Equal(true)) + Expect(isPodEvicted(*leaderPod)).To(BeTrue()) }) }) Describe("myOwnerRef", func() { @@ -130,17 +125,17 @@ var _ = Describe("Leader election", func() { It("should return an error when POD_NAME is not set", func() { os.Unsetenv("POD_NAME") _, err := myOwnerRef(context.TODO(), client, "") - Expect(err).ShouldNot(BeNil()) + Expect(err).Should(HaveOccurred()) }) It("should return an error if no pod is found", func() { os.Setenv("POD_NAME", "thisisnotthepodyourelookingfor") _, err := myOwnerRef(context.TODO(), client, "") - Expect(err).ShouldNot(BeNil()) + Expect(err).Should(HaveOccurred()) }) It("should return the owner reference without error", func() { os.Setenv("POD_NAME", "mypod") owner, err := myOwnerRef(context.TODO(), client, "testns") - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(owner.APIVersion).To(Equal("v1")) Expect(owner.Kind).To(Equal("Pod")) Expect(owner.Name).To(Equal("mypod")) @@ -163,17 +158,17 @@ var _ = Describe("Leader election", func() { It("should return an error when POD_NAME is not set", func() { os.Unsetenv("POD_NAME") _, err := getPod(context.TODO(), nil, "") - Expect(err).ShouldNot(BeNil()) + Expect(err).Should(HaveOccurred()) }) It("should return an error if no pod is found", func() { os.Setenv("POD_NAME", "thisisnotthepodyourelookingfor") _, err := getPod(context.TODO(), client, "") - Expect(err).ShouldNot(BeNil()) + Expect(err).Should(HaveOccurred()) }) It("should return the pod with the given name", func() { os.Setenv("POD_NAME", "mypod") pod, err := getPod(context.TODO(), client, "testns") - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pod).ShouldNot(BeNil()) Expect(pod.TypeMeta.APIVersion).To(Equal("v1")) Expect(pod.TypeMeta.Kind).To(Equal("Pod")) @@ -195,13 +190,11 @@ var _ = Describe("Leader election", func() { }) It("should return an error if no node is found", func() { node := corev1.Node{} - err := getNode(context.TODO(), client, "", &node) - Expect(err).ShouldNot(BeNil()) + Expect(getNode(context.TODO(), client, "", &node)).ToNot(Succeed()) }) It("should return the node with the given name", func() { node := corev1.Node{} - err := getNode(context.TODO(), client, "mynode", &node) - Expect(err).Should(BeNil()) + Expect(getNode(context.TODO(), client, "mynode", &node)).Should(Succeed()) Expect(node.TypeMeta.APIVersion).To(Equal("v1")) Expect(node.TypeMeta.Kind).To(Equal("Node")) }) @@ -228,33 +221,33 @@ var _ = Describe("Leader election", func() { It("should return false if node is invalid", func() { client = fake.NewClientBuilder().WithObjects().Build() ret := isNotReadyNode(context.TODO(), client, "") - Expect(ret).To(Equal(false)) + Expect(ret).To(BeFalse()) }) It("should return false if no NodeCondition is found", func() { client = fake.NewClientBuilder().WithObjects(node).Build() ret := isNotReadyNode(context.TODO(), client, nodeName) - Expect(ret).To(Equal(false)) + Expect(ret).To(BeFalse()) }) It("should return false if type is incorrect", func() { node.Status.Conditions[0].Type = corev1.NodeMemoryPressure node.Status.Conditions[0].Status = corev1.ConditionFalse client = fake.NewClientBuilder().WithObjects(node).Build() ret := isNotReadyNode(context.TODO(), client, nodeName) - Expect(ret).To(Equal(false)) + Expect(ret).To(BeFalse()) }) It("should return false if NodeReady's type is true", func() { node.Status.Conditions[0].Type = corev1.NodeReady node.Status.Conditions[0].Status = corev1.ConditionTrue client = fake.NewClientBuilder().WithObjects(node).Build() ret := isNotReadyNode(context.TODO(), client, nodeName) - Expect(ret).To(Equal(false)) + Expect(ret).To(BeFalse()) }) It("should return true when Type is set and Status is set to false", func() { node.Status.Conditions[0].Type = corev1.NodeReady node.Status.Conditions[0].Status = corev1.ConditionFalse client = fake.NewClientBuilder().WithObjects(node).Build() ret := isNotReadyNode(context.TODO(), client, nodeName) - Expect(ret).To(Equal(true)) + Expect(ret).To(BeTrue()) }) }) Describe("deleteLeader", func() { @@ -293,28 +286,23 @@ var _ = Describe("Leader election", func() { }) It("should return an error if existing is not found", func() { client = fake.NewClientBuilder().WithObjects(pod).Build() - err := deleteLeader(context.TODO(), client, pod, configmap) - Expect(err).ShouldNot(BeNil()) + Expect(deleteLeader(context.TODO(), client, pod, configmap)).ToNot(Succeed()) }) It("should return an error if pod is not found", func() { client = fake.NewClientBuilder().WithObjects(configmap).Build() - err := deleteLeader(context.TODO(), client, pod, configmap) - Expect(err).ShouldNot(BeNil()) + Expect(deleteLeader(context.TODO(), client, pod, configmap)).ToNot(Succeed()) }) It("should return an error if pod is nil", func() { client = fake.NewClientBuilder().WithObjects(pod, configmap).Build() - err := deleteLeader(context.TODO(), client, nil, configmap) - Expect(err).ShouldNot(BeNil()) + Expect(deleteLeader(context.TODO(), client, nil, configmap)).ToNot(Succeed()) }) It("should return an error if configmap is nil", func() { client = fake.NewClientBuilder().WithObjects(pod, configmap).Build() - err := deleteLeader(context.TODO(), client, pod, nil) - Expect(err).ShouldNot(BeNil()) + Expect(deleteLeader(context.TODO(), client, pod, nil)).ToNot(Succeed()) }) It("should return nil if pod and configmap exists and configmap's owner is the pod", func() { client = fake.NewClientBuilder().WithObjects(pod, configmap).Build() - err := deleteLeader(context.TODO(), client, pod, configmap) - Expect(err).Should(BeNil()) + Expect(deleteLeader(context.TODO(), client, pod, configmap)).To(Succeed()) }) }) diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index ba2b331..a680a11 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -40,11 +40,11 @@ var _ = Describe("Retrieving", func() { os.Setenv("HTTP_PROXY", "http_proxy_test") os.Setenv("NO_PROXY", "no_proxy_test") envVars := ReadProxyVarsFromEnv() - Expect(len(envVars)).To(Equal(6)) + Expect(envVars).To(HaveLen(6)) }) It("does not return unset variables", func() { envVars := ReadProxyVarsFromEnv() - Expect(len(envVars)).To(Equal(0)) + Expect(envVars).To(BeEmpty()) }) It("creates upper and lower case environment variables with the same value", func() { @@ -55,9 +55,9 @@ var _ = Describe("Retrieving", func() { for _, envName := range ProxyEnvNames { upperValue, err := checkValueFromEnvObj(envName, envVars) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) lowerValue, err := checkValueFromEnvObj(strings.ToLower(envName), envVars) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(upperValue).To(Equal(lowerValue)) } }) diff --git a/prune/prune_test.go b/prune/prune_test.go index 75e4893..9922eec 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -49,7 +49,7 @@ var _ = Describe("Prune", func() { ) BeforeEach(func() { testScheme, err := createSchemes() - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) fakeClient = crFake.NewClientBuilder().WithScheme(testScheme).Build() fakeObj = &corev1.Pod{} @@ -61,11 +61,11 @@ var _ = Describe("Prune", func() { Describe("Unprunable", func() { Describe("Error()", func() { It("Should Return a String Representation of Unprunable", func() { - unpruneable := Unprunable{ + unpruneable := &Unprunable{ Obj: &fakeObj, Reason: "TestReason", } - Expect(unpruneable.Error()).To(Equal(fmt.Sprintf("unable to prune %s: %s", client.ObjectKeyFromObject(fakeObj), unpruneable.Reason))) + Expect(unpruneable).To(MatchError(fmt.Sprintf("unable to prune %s: %s", client.ObjectKeyFromObject(fakeObj), unpruneable.Reason))) }) }) }) @@ -97,7 +97,7 @@ var _ = Describe("Prune", func() { Kind: "NotReal", }) - Expect(NewRegistry().IsPrunable(obj)).Should(BeNil()) + Expect(NewRegistry().IsPrunable(obj)).Should(Succeed()) }) }) @@ -106,7 +106,7 @@ var _ = Describe("Prune", func() { Describe("NewPruner()", func() { It("Should Return a New Pruner Object", func() { pruner, err := NewPruner(fakeClient, podGVK, myStrategy) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) }) @@ -118,7 +118,7 @@ var _ = Describe("Prune", func() { myStrategy, WithNamespace(namespace), WithLabels(labels)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) Expect(&pruner.registry).Should(Equal(DefaultRegistry())) Expect(pruner.namespace).Should(Equal(namespace)) @@ -131,8 +131,7 @@ var _ = Describe("Prune", func() { It("Should Error if schema.GroupVersionKind Parameter is empty", func() { // empty GVK struct pruner, err := NewPruner(fakeClient, schema.GroupVersionKind{}, myStrategy) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal("error when creating a new Pruner: gvk parameter can not be empty")) + Expect(err).Should(MatchError("error when creating a new Pruner: gvk parameter can not be empty")) Expect(pruner).Should(BeNil()) }) }) @@ -141,123 +140,109 @@ var _ = Describe("Prune", func() { Context("Does not return an Error", func() { testPruneWithDefaultIsPrunableFunc := func(gvk schema.GroupVersionKind) { pruner, err := NewPruner(fakeClient, gvk, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) - Expect(len(prunedObjects)).Should(Equal(2)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(prunedObjects).Should(HaveLen(2)) } It("Should Prune Pods with Default IsPrunableFunc", func() { // Create the test resources - in this case Pods - err := createTestPods(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestPods(fakeClient)).To(Succeed()) // Make sure the pod resources are properly created pods := &unstructured.UnstructuredList{} pods.SetGroupVersionKind(podGVK) - err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) - Expect(len(pods.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), pods)).To(Succeed()) + Expect(pods.Items).Should(HaveLen(3)) testPruneWithDefaultIsPrunableFunc(podGVK) // Get a list of the Pods to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) - Expect(len(pods.Items)).Should(Equal(1)) + Expect(fakeClient.List(context.Background(), pods)).To(Succeed()) + Expect(pods.Items).Should(HaveLen(1)) }) It("Should Prune Jobs with Default IsPrunableFunc", func() { // Create the test resources - in this case Jobs - err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestJobs(fakeClient)).To(Succeed()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) testPruneWithDefaultIsPrunableFunc(jobGVK) // Get a list of the job to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(1)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(1)) }) It("Should Remove Resource When Using a Custom IsPrunableFunc", func() { // Create the test resources - in this case Jobs - err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestJobs(fakeClient)).To(Succeed()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) // Register our custom IsPrunableFunc RegisterIsPrunableFunc(jobGVK, myIsPrunable) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) - Expect(len(prunedObjects)).Should(Equal(2)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(prunedObjects).Should(HaveLen(2)) // Get a list of the jobs to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(1)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(1)) }) It("Should Not Prune Resources when using a DryRunClient", func() { // Create the test resources - in this case Pods - err := createTestPods(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestPods(fakeClient)).To(Succeed()) // Make sure the pod resources are properly created pods := &unstructured.UnstructuredList{} pods.SetGroupVersionKind(podGVK) - err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) - Expect(len(pods.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), pods)).To(Succeed()) + Expect(pods.Items).Should(HaveLen(3)) dryRunClient := client.NewDryRunClient(fakeClient) pruner, err := NewPruner(dryRunClient, podGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) - Expect(len(prunedObjects)).Should(Equal(2)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(prunedObjects).Should(HaveLen(2)) // Get a list of the Pods to make sure we haven't pruned any - err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) - Expect(len(pods.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), pods)).To(Succeed()) + Expect(pods.Items).Should(HaveLen(3)) }) It("Should Skip Pruning a Resource If IsPrunable Returns an Error of Type Unprunable", func() { // Create the test resources - in this case Jobs - err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestJobs(fakeClient)).To(Succeed()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that throws Unprunable error @@ -272,31 +257,28 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, errorPrunableFunc) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) - Expect(len(prunedObjects)).Should(Equal(0)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(prunedObjects).Should(BeEmpty()) // Get a list of the jobs to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) }) }) Context("Returns an Error", func() { It("Should Return an Error if IsPrunableFunc Returns an Error That is not of Type Unprunable", func() { // Create the test resources - in this case Jobs - err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestJobs(fakeClient)).To(Succeed()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that throws non Unprunable error @@ -308,27 +290,23 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, errorPrunableFunc) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal("TEST")) - Expect(len(prunedObjects)).Should(Equal(0)) + Expect(err).Should(MatchError("TEST")) + Expect(prunedObjects).Should(BeEmpty()) // Get a list of the jobs to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) }) It("Should Return An Error If Strategy Function Returns An Error", func() { // Create the test resources - in this case Jobs - err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestJobs(fakeClient)).To(Succeed()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) // strategy that will return an error prunerStrategy := func(ctx context.Context, objs []client.Object) ([]client.Object, error) { @@ -336,37 +314,33 @@ var _ = Describe("Prune", func() { } pruner, err := NewPruner(fakeClient, jobGVK, prunerStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) // Register our custom IsPrunableFunc RegisterIsPrunableFunc(jobGVK, myIsPrunable) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal("error determining prunable objects: TESTERROR")) + Expect(err).Should(MatchError("error determining prunable objects: TESTERROR")) Expect(prunedObjects).Should(BeNil()) // Get a list of the jobs to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) }) It("Should Return an Error if it can not Prune a Resource", func() { // Create the test resources - in this case Jobs - err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) + Expect(createTestJobs(fakeClient)).To(Succeed()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(3)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(HaveLen(3)) pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that returns nil but also deletes the object @@ -380,14 +354,12 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, prunableFunc) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(ContainSubstring("error pruning object: jobs.batch \"churro1\" not found")) - Expect(len(prunedObjects)).Should(Equal(0)) + Expect(err).Should(MatchError(ContainSubstring("error pruning object: jobs.batch \"churro1\" not found"))) + Expect(prunedObjects).Should(BeEmpty()) // Get a list of the jobs to make sure we have pruned the ones we expected - err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) - Expect(len(jobs.Items)).Should(Equal(0)) + Expect(fakeClient.List(context.Background(), jobs)).To(Succeed()) + Expect(jobs.Items).Should(BeEmpty()) }) }) @@ -396,7 +368,7 @@ var _ = Describe("Prune", func() { Describe("GVK()", func() { It("Should return the GVK field in the Pruner", func() { pruner, err := NewPruner(fakeClient, podGVK, myStrategy) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) Expect(pruner.GVK()).Should(Equal(podGVK)) }) @@ -405,7 +377,7 @@ var _ = Describe("Prune", func() { Describe("Labels()", func() { It("Should return the Labels field in the Pruner", func() { pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithLabels(appLabels)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) Expect(pruner.Labels()).Should(Equal(appLabels)) }) @@ -414,7 +386,7 @@ var _ = Describe("Prune", func() { Describe("Namespace()", func() { It("Should return the Namespace field in the Pruner", func() { pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithNamespace(namespace)) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(pruner).ShouldNot(BeNil()) Expect(pruner.Namespace()).Should(Equal(namespace)) }) @@ -437,8 +409,7 @@ var _ = Describe("Prune", func() { pod.SetGroupVersionKind(podGVK) // Run it through DefaultPodIsPrunable - err := DefaultPodIsPrunable(pod) - Expect(err).Should(BeNil()) + Expect(DefaultPodIsPrunable(pod)).To(Succeed()) }) It("Should Panic When client.Object is not of type 'Pod'", func() { @@ -467,12 +438,11 @@ var _ = Describe("Prune", func() { // Run it through DefaultPodIsPrunable err := DefaultPodIsPrunable(pod) - Expect(err).ShouldNot(BeNil()) + Expect(err).Should(MatchError(fmt.Sprintf("unable to prune %s: Pod has not succeeded", client.ObjectKeyFromObject(pod)))) var expectErr *Unprunable Expect(errors.As(err, &expectErr)).Should(BeTrue()) Expect(expectErr.Reason).Should(Equal("Pod has not succeeded")) Expect(expectErr.Obj).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal(fmt.Sprintf("unable to prune %s: Pod has not succeeded", client.ObjectKeyFromObject(pod)))) }) }) @@ -492,8 +462,7 @@ var _ = Describe("Prune", func() { job.SetGroupVersionKind(jobGVK) // Run it through DefaultJobIsPrunable - err := DefaultJobIsPrunable(job) - Expect(err).Should(BeNil()) + Expect(DefaultJobIsPrunable(job)).To(Succeed()) }) It("Should Return An Error When Kind Is Not 'Job'", func() { @@ -522,12 +491,11 @@ var _ = Describe("Prune", func() { // Run it through DefaultJobIsPrunable err := DefaultJobIsPrunable(job) - Expect(err).ShouldNot(BeNil()) + Expect(err).Should(MatchError(ContainSubstring(fmt.Sprintf("unable to prune %s: Job has not completed", client.ObjectKeyFromObject(job))))) var expectErr *Unprunable Expect(errors.As(err, &expectErr)).Should(BeTrue()) Expect(expectErr.Reason).Should(Equal("Job has not completed")) Expect(expectErr.Obj).ShouldNot(BeNil()) - Expect(err.Error()).Should(ContainSubstring(fmt.Sprintf("unable to prune %s: Job has not completed", client.ObjectKeyFromObject(job)))) }) }) @@ -535,13 +503,13 @@ var _ = Describe("Prune", func() { resources := createDatedResources() It("Should return the 3 oldest resources", func() { resourcesToRemove, err := NewPruneByCountStrategy(2)(context.Background(), resources) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(resourcesToRemove).Should(Equal(resources[2:])) }) It("Should return nil", func() { resourcesToRemove, err := NewPruneByCountStrategy(5)(context.Background(), resources) - Expect(err).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) Expect(resourcesToRemove).Should(BeNil()) }) }) @@ -551,15 +519,15 @@ var _ = Describe("Prune", func() { It("Should return 2 resources", func() { date := time.Now().Add(time.Hour * time.Duration(2)) resourcesToRemove, err := NewPruneByDateStrategy(date)(context.Background(), resources) - Expect(err).Should(BeNil()) - Expect(len(resourcesToRemove)).Should(Equal(2)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(resourcesToRemove).Should(HaveLen(2)) }) It("Should return 0 resources", func() { date := time.Now().Add(time.Hour * time.Duration(24)) resourcesToRemove, err := NewPruneByDateStrategy(date)(context.Background(), resources) - Expect(err).Should(BeNil()) - Expect(len(resourcesToRemove)).Should(Equal(0)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(resourcesToRemove).Should(BeEmpty()) }) })