Fix deregister_targets e2etest #491
Conversation
zijun726911
force-pushed
the
fix-e2e-test
branch
from
6c90f3c to
c0e3dbf
Compare
| Equal(vpclattice.TargetStatusInitial), | ||
| Equal(vpclattice.TargetStatusHealthy), | ||
| )) | ||
| } |
There was a problem hiding this comment.
g.Expect(len(targets)).To(BeEquivalentTo(*deployments[i].Spec.Replicas))
Old code forget to do assertion on the retrieved targets length, That could cause before setup complete and the targets haven't yet being created, it could wrongly slips to the below 2 test cases. And the verifyNoTargetsForTargetGroup() just pass not because of the controller "Kubernetes Deployment deletion triggers targets de-registering" behavior but due to test setup incomplete.
|
|
||
| It("Kubernetes Deployment deletion triggers targets de-registering", func() { | ||
| testFramework.ExpectDeleted(ctx, services[1]) | ||
| testFramework.ExpectDeleted(ctx, deployments[1]) |
There was a problem hiding this comment.
Should be a typo. this test intend to do: "Kubernetes Deployment deletion triggers targets de-registering"
zijun726911
force-pushed
the
fix-e2e-test
branch
from
c0e3dbf to
50f47d8
Compare
| Equal(vpclattice.TargetStatusHealthy), | ||
| )) | ||
| } | ||
| }).Should(Succeed()) |
There was a problem hiding this comment.
please add timeout, polling rate
| g.Expect(*target.Port).To(BeEquivalentTo(service.Spec.Ports[0].TargetPort.IntVal)) | ||
| g.Expect(*target.Status).To(Or( | ||
| Equal(vpclattice.TargetStatusInitial), | ||
| Equal(vpclattice.TargetStatusHealthy), | ||
| )) |
There was a problem hiding this comment.
why you need any of these?
There was a problem hiding this comment.
Need to do some assertions to confirm lattice targets setup success. otherwise below test cases could wrong pass due to incomplete pre-test setup
There was a problem hiding this comment.
should we wait for Healthy status only? If we succeed on TargetStatusInitial, it's almost same as not doing assertions
2 participants
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.