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.

Sign up for GitHub

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

Jump to bottom

[scheduler cleanup phase 1]: Refactor factory_test.go to use a fake k8s client #68963

Closed
misterikkit opened this issue Sep 21, 2018 · 9 comments · Fixed by #69412
Closed

[scheduler cleanup phase 1]: Refactor factory_test.go to use a fake k8s client #68963

misterikkit opened this issue Sep 21, 2018 · 9 comments · Fixed by #69412
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling.

Comments

@misterikkit
Copy link

misterikkit commented Sep 21, 2018 •
edited
Loading

PLEASE COORDINATE WITH SIG-SCHEDULING BEFORE WORKING ON THIS ISSUE. We want to avoid the situation where two people accidentally start work on the same fix.
Main tracking issue: #68951

The tests in pkg/scheduler/factory/factory_test.go depend on mocking REST/HTTP with the utils in pkg/scheduler/testing. It would be much simpler to use k8s.io/client-go/kubernetes/fake.Clientset instead. None of the behavior under test depends on how the REST and HTTP stuff behaves.

/ref #68962
/sig scheduling
/kind cleanup
/help
@k8s-ci-robot k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Sep 21, 2018
This was referenced Sep 21, 2018
Closed
Closed
@shubheksha
Copy link
Contributor

I'm working on a very similar fix for service-catalog, so I can pick this up as well!

@shubheksha
Copy link
Contributor

Ref kubernetes-retired/service-catalog#2358

@k8s-ci-robot
Copy link
Contributor

@tossmilestone: GitHub didn't allow me to assign the following users: tossmilestone.

Note that only kubernetes members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign

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/test-infra repository.

@shubheksha
Copy link
Contributor

@tossmilestone, are you interested in working on this issue?

@tossmilestone
Copy link
Member

@shubheksha yes, if you are not working on it, I will have a try. Thanks!

@misterikkit
Copy link
Author

@tossmilestone Have you started this? How is it going?
/assign @tossmilestone
/remove-help

@k8s-ci-robot k8s-ci-robot removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Oct 1, 2018
@k8s-ci-robot
Copy link
Contributor

@misterikkit: GitHub didn't allow me to assign the following users: tossmilestone.

Note that only kubernetes members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

@tossmilestone Have you started this? How is it going?
/assign @tossmilestone
/remove-help

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/test-infra repository.

@tossmilestone
Copy link
Member

@misterikkit, I am now working on it. It will be done later.

@tossmilestone tossmilestone mentioned this issue Oct 4, 2018
Merged
@tossmilestone
Copy link
Member

@misterikkit This Issue has been done, PTAL. See #69412.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor scheduler factory test tossmilestone/kubernetes
4 participants
@tossmilestone @misterikkit @shubheksha @k8s-ci-robot