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 14, 2021
1 parent 42c8772 commit e18b016
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
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
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
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 e18b016

Please sign in to comment.