Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

route controller unit tests #264

Merged
merged 1 commit into from
Dec 29, 2017

Conversation

andrewsykim
Copy link
Collaborator

Adds unit tests for route controller.

cc @murali-reddy

#229

@@ -93,7 +95,15 @@ func StartServiceWatcher(clientset *kubernetes.Clientset, resyncPeriod time.Dura

svcw.clientset = clientset
svcw.broadcaster = utils.NewBroadcaster()
lw := cache.NewListWatchFromClient(clientset.Core().RESTClient(), "services", metav1.NamespaceAll, fields.Everything())
lw := &cache.ListWatch{
Copy link
Collaborator Author

@andrewsykim andrewsykim Dec 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to switch to this since the fake client does not have an implementation for clientset.Core().RESTClient(). It should effectively be the same thing but I have not tested it this on a real cluster.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a related note, I don't think we should be using global variable for the watchers, but something that can be addressed later.

@andrewsykim andrewsykim force-pushed the routes-unit-test branch 2 times, most recently from 060d89f to fa27d8f Compare December 28, 2017 21:17
@murali-reddy murali-reddy merged commit 4eca430 into cloudnativelabs:master Dec 29, 2017
@murali-reddy
Copy link
Member

Thanks @andrewsykim i have tested againest real cluster, switch from cache.NewListWatchFromClient to cache.ListWatch did not have any impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants