-
Notifications
You must be signed in to change notification settings - Fork 41.9k
Description
What happened?
Originally from #121861 (comment)
There was a bug #121860 that caused test to fail, and gomega.Consistenly was used to replace the code but requires more updates to the testing code to be used as intended.
When using gomega.Consistently, you are making a statement about some observable fact. The callback should only deliver that fact, the statement then should be (no pun intended) in the Should. That way you get all the advantages listed in https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/writing-good-e2e-tests.md#polling-and-timeouts.
You also need to check error handling. Does
rc.GetReplicasfail on errors? It would be better to return an error and letConsistentlyandHandleRetrydeal with it.
It fails hard with in its helper function after making the API call. If I understand correctly that isn't following the pattern suggested in https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/writing-good-e2e-tests.md#polling-and-timeouts. either.
kubernetes/test/e2e/framework/autoscaling/autoscaling_utils.go
Lines 444 to 452 in 1bb2773
| func (rc *ResourceConsumer) GetReplicas(ctx context.Context) int { | |
| switch rc.kind { | |
| case KindRC: | |
| replicationController, err := rc.clientSet.CoreV1().ReplicationControllers(rc.nsName).Get(ctx, rc.name, metav1.GetOptions{}) | |
| framework.ExpectNoError(err) | |
| if replicationController == nil { | |
| framework.Failf(rcIsNil) | |
| } | |
| return int(replicationController.Status.ReadyReplicas) |
What did you expect to happen?
Tests to follow recommended guidance.
How can we reproduce it (as minimally and precisely as possible)?
See discussion in #121861 (comment)
Anything else we need to know?
/kind cleanup
/sig autoscaling
/sig testing
Kubernetes version
$ kubectl version
# paste output hereCloud provider
OS version
# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here
# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here