Skip to content

Commit

Permalink
Fix failing unit test
Browse files Browse the repository at this point in the history 
Also cleanup and standardize test phrasing and whitespace

[Issue #462]

Authored-by: Matt Royal <mroyal@vmware.com>
  • Loading branch information
@matt-royal
matt-royal committed Jan 27, 2022
1 parent 7aa0255 commit 52c7d91
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 27 deletions.
2 changes: 1 addition & 1 deletion controllers/controllers/workloads/cfprocess_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (r *CFProcessReconciler) createOrPatchLRP(ctx context.Context, cfApp *workl

_, err = controllerutil.CreateOrPatch(ctx, r.Client, actualLRP, lrpMutateFunction(actualLRP, desiredLRP))
if err != nil {
r.Log.Error(err, "Error calling CreateOrPatch on lrp")
r.Log.Error(err, "Error calling CreateOrPatch on LRP")
return err
}
return nil
Expand Down
61 changes: 35 additions & 26 deletions controllers/controllers/workloads/cfprocess_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@ import (

networkingv1alpha1 "code.cloudfoundry.org/cf-k8s-controllers/controllers/apis/networking/v1alpha1"

workloadsv1alpha1 "code.cloudfoundry.org/cf-k8s-controllers/controllers/apis/workloads/v1alpha1"
. "code.cloudfoundry.org/cf-k8s-controllers/controllers/controllers/workloads"
"code.cloudfoundry.org/cf-k8s-controllers/controllers/controllers/workloads/fake"
. "code.cloudfoundry.org/cf-k8s-controllers/controllers/controllers/workloads/testutils"
eiriniv1 "code.cloudfoundry.org/eirini-controller/pkg/apis/eirini/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"

workloadsv1alpha1 "code.cloudfoundry.org/cf-k8s-controllers/controllers/apis/workloads/v1alpha1"
. "code.cloudfoundry.org/cf-k8s-controllers/controllers/controllers/workloads"
"code.cloudfoundry.org/cf-k8s-controllers/controllers/controllers/workloads/fake"
. "code.cloudfoundry.org/cf-k8s-controllers/controllers/controllers/workloads/testutils"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -131,6 +132,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
},
}
cfServiceInstanceError = nil
lrp = nil
lrpError = nil
lrpListError = nil

Expand Down Expand Up @@ -158,7 +160,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
cfServiceInstance.DeepCopyInto(obj)
return cfServiceInstanceError
case *eiriniv1.LRP:
if lrpError == nil {
if lrp != nil && lrpError == nil {
lrp.DeepCopyInto(obj)
}
return lrpError
Expand Down Expand Up @@ -214,10 +216,12 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
Expect(reconcileErr).ToNot(HaveOccurred())
})

It("does not attempt to create any new LRPs", func() {
Expect(fakeClient.CreateCallCount()).To(Equal(0), "Client.Create call count mismatch")
})
})

When("the CFApp is updated from desired state STARTED to STOPPED", func() {
BeforeEach(func() {
cfApp.Spec.DesiredState = workloadsv1alpha1.StoppedState
Expand Down Expand Up @@ -248,13 +252,15 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
Expect(reconcileErr).ToNot(HaveOccurred())
})

It("deletes any existing LRPs for the CFApp", func() {
Expect(fakeClient.DeleteCallCount()).To(Equal(1), "Client.Delete call count mismatch")
})
})

When("the CFApp is started and there are existing routes matching", func() {
const testPort = 1234

BeforeEach(func() {
cfApp.Spec.DesiredState = workloadsv1alpha1.StartedState
lrpError = apierrors.NewNotFound(schema.GroupResource{}, "some-guid")
Expand Down Expand Up @@ -302,7 +308,8 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
},
}
})
It("Chooses the oldest matching route", func() {

It("chooses the oldest matching route", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
Expect(reconcileErr).ToNot(HaveOccurred())

Expand All @@ -318,23 +325,25 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
BeforeEach(func() {
cfApp.Spec.DesiredState = workloadsv1alpha1.StartedState
})

When("fetch CFProcess returns an error", func() {
BeforeEach(func() {
cfProcessError = errors.New(failsOnPurposeErrorMessage)
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(failsOnPurposeErrorMessage))
})
})

When("fetch CFProcess returns a NotFoundError", func() {
BeforeEach(func() {
cfProcessError = apierrors.NewNotFound(schema.GroupResource{}, cfProcess.Name)
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should NOT return an error", func() {
It("doesn't return an error", func() {
Expect(reconcileErr).NotTo(HaveOccurred())
})
})
Expand All @@ -345,18 +354,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
Expect(reconcileErr).To(MatchError(failsOnPurposeErrorMessage))
})
})

When("fetch LRPList returns an error", func() {
BeforeEach(func() {
lrpListError = errors.New(failsOnPurposeErrorMessage)
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(failsOnPurposeErrorMessage))
})
})
Expand All @@ -367,7 +365,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(failsOnPurposeErrorMessage))
})
})
Expand All @@ -378,7 +376,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError("no build droplet status on CFBuild"))
})
})
Expand All @@ -389,7 +387,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(failsOnPurposeErrorMessage))
})
})
Expand All @@ -400,7 +398,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(ContainSubstring(failsOnPurposeErrorMessage)))
})
})
Expand All @@ -411,7 +409,7 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(ContainSubstring(failsOnPurposeErrorMessage)))
})
})
Expand All @@ -422,11 +420,22 @@ var _ = Describe("CFProcessReconciler Unit Tests", func() {
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("should return an error", func() {
It("returns an error", func() {
Expect(reconcileErr).To(MatchError(ContainSubstring(failsOnPurposeErrorMessage)))
})
})

When("fetch LRPList returns an error", func() {
BeforeEach(func() {
lrpListError = errors.New(failsOnPurposeErrorMessage)
_, reconcileErr = cfProcessReconciler.Reconcile(ctx, req)
})

It("returns an error", func() {
Expect(reconcileErr).To(MatchError(failsOnPurposeErrorMessage))
})
})

})
})
})

0 comments on commit 52c7d91

Please sign in to comment.