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

Respect optional param for ConfigMaps and Secrets #562

Merged
merged 3 commits into from
Jan 28, 2020

Conversation

tomislater
Copy link
Contributor

@tomislater tomislater commented Jan 24, 2020

I am working on tests, but it might take some time :) I am new here.

#555

@tomislater tomislater changed the title #555 Respect optional param for ConfigMaps and Secrets Respect optional param for ConfigMaps and Secrets Jan 24, 2020
@zroubalik
Copy link
Member

LGTM, thanks @tomislater

@tomislater
Copy link
Contributor Author

tomislater commented Jan 24, 2020

Uhh, I do not know how to start writing tests here for something other than scalers.
Handlers do not have any tests currently.

I wanted to use fake client like here or here. But, I have got this error:

pkg/handler/scale_handler_test.go:22:2: cannot use "k8s.io/client-go/kubernetes/fake".NewSimpleClientset() (type *"k8s.io/client-go/kubernetes/fake".Clientset) as type client.Client in field value:
        *"k8s.io/client-go/kubernetes/fake".Clientset does not implement client.Client (missing Create method)

when I want to create test ScaleHandler:

var testScaleHandler = ScaleHandler{
	client:           testclient.NewSimpleClientset(),
	reconcilerScheme: scheme.Scheme,
}

Do you have any suggestions? Or maybe someone is working on writing tests for handlers?

I have tested it on my cluster and it works, but still... :P

@ahmelsayed
Copy link
Contributor

We're using operator-sdk for interacting with kuberentes, so you'll have to create the fake client like this https://github.com/operator-framework/operator-sdk/blob/master/doc/user/unit-testing.md

That part is currently only partially covered by the integration tests under tests/

@tomislater
Copy link
Contributor Author

Thanks, I am going to add tests.

@msftclas
Copy link

msftclas commented Jan 28, 2020

CLA assistant check
All CLA requirements met.

@ahmelsayed
Copy link
Contributor

Thank you very much @tomislater

@ahmelsayed ahmelsayed merged commit 16cae71 into kedacore:master Jan 28, 2020
preflightsiren pushed a commit to preflightsiren/keda that referenced this pull request Nov 7, 2021
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.

4 participants