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

Unable to use FakeScaleClient in tests #56865

Closed
p0lyn0mial opened this Issue Dec 5, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@p0lyn0mial
Contributor

p0lyn0mial commented Dec 5, 2017

Is this a BUG REPORT or FEATURE REQUEST?:

Uncomment only one, leave it on its own line:

/kind bug

/kind feature

What happened:
While implementing generic scaler in kubectl I wanted to mock scale client in order to write some unit tests. My initial idea was to employ FakeScaleClient, so I extended existing code and registered desired types in the Scheme. Later on I realised that the scale client doesn't accept version. Every call made from within my test ended up with an error saying "no registered kinds for". Which makes sense since the Scheme holds very specific gvks

One way of resolving this issue would be to provide a RESTMapper that would resolve to proper gvk just before the actual look up in the Scheme. But it turned out that we don't have a fake RESTMapper or I couldn't find one.

Another idea would be to derive RESTMapper from fake discovery but this doesn't seem to be viable approach.

As a workaround I created scale client from scratch in my unit test. Once this issue is resolved the test could be adapted to the new approach.

@p0lyn0mial

This comment has been minimized.

Show comment
Hide comment
@p0lyn0mial

p0lyn0mial Dec 5, 2017

Contributor

/sig api-machinery

Contributor

p0lyn0mial commented Dec 5, 2017

/sig api-machinery

@yliaog

This comment has been minimized.

Show comment
Hide comment
@yliaog

yliaog Dec 7, 2017

Contributor
Contributor

yliaog commented Dec 7, 2017

@DirectXMan12

This comment has been minimized.

Show comment
Hide comment
@DirectXMan12

DirectXMan12 Dec 9, 2017

Contributor

/assign @DirectXMan12

Contributor

DirectXMan12 commented Dec 9, 2017

/assign @DirectXMan12

@DirectXMan12

This comment has been minimized.

Show comment
Hide comment
@DirectXMan12

DirectXMan12 Dec 9, 2017

Contributor

We do actually have an effective equivalent of a fake rest mapper, I've used it before. I just have to dig up the right pattern.

Contributor

DirectXMan12 commented Dec 9, 2017

We do actually have an effective equivalent of a fake rest mapper, I've used it before. I just have to dig up the right pattern.

@p0lyn0mial

This comment has been minimized.

Show comment
Hide comment
@p0lyn0mial

p0lyn0mial Feb 26, 2018

Contributor

@DirectXMan12 any update on that :) ?

Contributor

p0lyn0mial commented Feb 26, 2018

@DirectXMan12 any update on that :) ?

@DirectXMan12

This comment has been minimized.

Show comment
Hide comment
@DirectXMan12

DirectXMan12 Mar 1, 2018

Contributor

Having gone through, I'm unclear on what the issue is here. This is an example that converts the current code to use the built-in scale client: p0lyn0mial#2

Contributor

DirectXMan12 commented Mar 1, 2018

Having gone through, I'm unclear on what the issue is here. This is an example that converts the current code to use the built-in scale client: p0lyn0mial#2

@p0lyn0mial

This comment has been minimized.

Show comment
Hide comment
@p0lyn0mial

p0lyn0mial Mar 19, 2018

Contributor

apparently, I didn't know how to use fake client. Thanks for showing how to do this, I'm closing the issue.

Contributor

p0lyn0mial commented Mar 19, 2018

apparently, I didn't know how to use fake client. Thanks for showing how to do this, I'm closing the issue.

@p0lyn0mial p0lyn0mial closed this Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment