test(kubernetes): add WatchTargets test for kubeconfig changes #531
Conversation
Signed-off-by: Marc Nuri <[email protected]>
| close(callbackInvoked) | ||
| }) | ||
| return nil | ||
| }) |
There was a problem hiding this comment.
There is no guarantee here that the watcher has successfully started listening for events. I feel like this might introduce a race?
There was a problem hiding this comment.
This is a utility callback for the tests (not production code).
It's used to wait for the callback to be invoked and then proceed with the assertions.
If the watchers haven't started (or any other issue) the callback will never be invoked and the tests will fail after the provided timeout.
I haven't seen any concurrency issues so far.
There was a problem hiding this comment.
OK, thinking it better, I understand that maybe you're referring that the fn() is invoked without guarantees of watchers having started.
For the watcher.Kubeconfig the code is synchronous and shouldn't be an issue, the watch is started by the time the method returns.
For watcher.ClusterState the code is not synchronous, but shouldn't be an issue either.
In all cases, the code executed in the enclosed fn() is what unblocks the callbackInvoked select.
If there's a race condition the test simply won't pass.
|
Closing in favor of #532 which addresses the issue too. |
2 participants
Relates to #522