🌱 Test cache reader waits for cache sync#3434
🌱 Test cache reader waits for cache sync#3434k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Conversation
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
size/M
pkg/cache/cache_test.go
Outdated
| g.Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| services := &corev1.ServiceList{} | ||
| listCtx, listCtxcancel := context.WithTimeout(ctx, time.Second) |
There was a problem hiding this comment.
The sync check internally works through polling (We've inherited that from upstream). I both want to keep the timeout high enough to be sure it polls a few times even if maybe in the future the poll interval changes but also not have the test actually run for poll interval duration, hence using synctest.
| g.Expect(c.Start(ctx)).To(Succeed()) | ||
| close(cacheDone) | ||
| }() | ||
| synctest.Wait() // Let the cache finish starting |
There was a problem hiding this comment.
Coordinating "cache has finished starting up" with the rest would require us to sleep without synctest, as Start is blocking
|
/cherrypick release-0.23 |
|
@alvaroaleman: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
alvaroaleman
force-pushed
the
testcachesync
branch
2 times, most recently
from
d7c57dc to
72ef016
Compare
We don't seem to have any test coverage of this at all, change that.
alvaroaleman
force-pushed
the
testcachesync
branch
from
72ef016 to
717f6a7
Compare
|
Thx! /lgtm Feel free to remove hold |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. DetailsGit tree hash: f3d5bd97f14761a09a7f430e03418cb2bca122fd |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
@alvaroaleman: new pull request created: #3438 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
We don't seem to have any test coverage of this at all, change that.
/hold