Skip to content

Commit

Permalink
Correct unit tests falsely succeeding
Browse files Browse the repository at this point in the history
These tests were not preparing the test objects correctly: they only updated
them in memory but not on the fake client. This wasn't caught until now
because the fake client mimicked the real json decoder, which didn't unset
fields not present on the server. Now that the fake client zeroes fields,
the tests started failing (which is correct). So fix the tests.

ref kubernetes-sigs/controller-runtime#1651
  • Loading branch information
timebertt committed Oct 12, 2021
1 parent a0dfb71 commit dea9ca1
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 24 deletions.
45 changes: 25 additions & 20 deletions pkg/operation/botanist/clusteridentity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -73,11 +72,6 @@ var _ = Describe("ClusterIdentity", func() {
ctrl = gomock.NewController(GinkgoT())
clusterIdentity = mockclusteridentity.NewMockInterface(ctrl)

s := runtime.NewScheme()
Expect(corev1.AddToScheme(s)).To(Succeed())
Expect(extensionsv1alpha1.AddToScheme(s)).NotTo(HaveOccurred())
Expect(gardencorev1beta1.AddToScheme(s))

shoot = &gardencorev1beta1.Shoot{
ObjectMeta: metav1.ObjectMeta{
Name: shootName,
Expand All @@ -87,6 +81,13 @@ var _ = Describe("ClusterIdentity", func() {
UID: shootUID,
},
}
})

JustBeforeEach(func() {
s := runtime.NewScheme()
Expect(corev1.AddToScheme(s)).To(Succeed())
Expect(extensionsv1alpha1.AddToScheme(s)).NotTo(HaveOccurred())
Expect(gardencorev1beta1.AddToScheme(s))

cluster := &extensionsv1alpha1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -125,27 +126,31 @@ var _ = Describe("ClusterIdentity", func() {
ctrl.Finish()
})

DescribeTable("#EnsureShootClusterIdentity",
func(mutator func()) {
mutator()

Describe("#EnsureShootClusterIdentity", func() {
test := func() {
Expect(botanist.EnsureShootClusterIdentity(ctx)).NotTo(HaveOccurred())

Expect(gardenClient.Get(ctx, kutil.Key(shootNamespace, shootName), shoot)).To(Succeed())
Expect(shoot.Status.ClusterIdentity).NotTo(BeNil())
Expect(*shoot.Status.ClusterIdentity).To(Equal(expectedShootClusterIdentity))
},

Entry("cluster identity is nil", func() {
shoot.Status.ClusterIdentity = nil
}),
Entry("cluster idenitty already exists", func() {
shoot.Status.ClusterIdentity = pointer.String(expectedShootClusterIdentity)
}),
)
}

Context("cluster identity is nil", func() {
BeforeEach(func() {
shoot.Status.ClusterIdentity = nil
})
It("should set shoot.status.clusterIdentity", test)
})
Context("cluster identity already exists", func() {
BeforeEach(func() {
shoot.Status.ClusterIdentity = pointer.String(expectedShootClusterIdentity)
})
It("should not touch shoot.status.clusterIdentity", test)
})
})

Describe("#DeployClusterIdentity", func() {
BeforeEach(func() {
JustBeforeEach(func() {
botanist.Shoot.GetInfo().Status.ClusterIdentity = &expectedShootClusterIdentity
clusterIdentity.EXPECT().SetIdentity(expectedShootClusterIdentity)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ var _ = Describe("#Service", func() {
It("waits for service", func() {
Expect(defaultDepWaiter.Deploy(ctx)).To(Succeed())

key := client.ObjectKeyFromObject(expected)
Expect(c.Get(ctx, key, expected)).To(Succeed())

expected.Status = corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{{IP: "3.3.3.3"}},
},
}

key := client.ObjectKeyFromObject(expected)
Expect(c.Get(ctx, key, expected)).To(Succeed())
Expect(c.Status().Update(ctx, expected)).To(Succeed())
Expect(defaultDepWaiter.Wait(ctx)).To(Succeed())
Expect(ingressIP).To(Equal("3.3.3.3"))
Expand Down
4 changes: 3 additions & 1 deletion pkg/utils/gardener/deletion_confirmation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import (
. "github.com/gardener/gardener/pkg/utils/gardener"
"github.com/gardener/gardener/pkg/utils/test"
. "github.com/gardener/gardener/pkg/utils/test/matchers"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

var _ = Describe("DeletionConfirmation", func() {
Expand Down Expand Up @@ -109,6 +109,8 @@ var _ = Describe("DeletionConfirmation", func() {
mockNow.EXPECT().Do().Return(now.UTC()).AnyTimes()

obj.SetAnnotations(map[string]string{"foo": "bar"})
Expect(c.Update(ctx, obj)).To(Succeed())

expectedAnnotations := map[string]string{"foo": "bar", ConfirmationDeletion: "true", v1beta1constants.GardenerTimestamp: now.UTC().String()}

Expect(ConfirmDeletion(ctx, c, obj)).To(Succeed())
Expand Down

0 comments on commit dea9ca1

Please sign in to comment.