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

Add --local-operator-flags for testing with --up-local #1509

Conversation

nikhil-thomas
Copy link
Contributor

@nikhil-thomas nikhil-thomas commented Jun 1, 2019

operator-sdk test : add --local-operator-flags to operator-sdk test local --up-local command

why this change was made: without this flag, we cannot write tests to test operator code that uses command line flags. A flag like this is already there in operator-sdk up local command (--operator-flags).

Fixes #1476

Description of the change:

This patch adds --local-operator-flags to
operator-sdk test local --up-local command

It is provides the functionality provided by
--operator-flags flag in operator-sdk up local --operator-flags command.

Motivation for the change:

At present there is no way to pass command line flags to an operator
while testing using operator-sdk test local --up-local command.

There is a flag --go-test-flags but it cannot be used to pass flags to
the operator code being tested using --up-local flag

Signed-off-by: Nikhil Thomas <nikthoma@redhat.com >

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 1, 2019
@nikhil-thomas nikhil-thomas changed the title Add --local-operator-flags in for testing with --up-local Add --local-operator-flags in for testing with --up-local Jun 1, 2019
@nikhil-thomas nikhil-thomas changed the title Add --local-operator-flags in for testing with --up-local Add --local-operator-flags for testing with --up-local Jun 1, 2019
@nikhil-thomas nikhil-thomas force-pushed the test_--up-local_runtime-argument-flag branch 2 times, most recently from e87e94f to 989c646 Compare June 2, 2019 03:40
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 2, 2019
@nikhil-thomas nikhil-thomas changed the title Add --local-operator-flags for testing with --up-local Add --operator-flags for testing with --up-local Jun 2, 2019
@nikhil-thomas nikhil-thomas force-pushed the test_--up-local_runtime-argument-flag branch from 989c646 to e0fbbb5 Compare June 2, 2019 04:09
This patch adds `--local-operator-flags` to
`operator-sdk test local --up-local` command

It is prvides the functionalisty provided by
`--operator-flags` flag in `operator-sdk up local --operator-flags` command.

At present there is no way to pass command line flags to an operator
while testing using `operator-sdk test local --up-local` command.

There is a flag `--go-test-flags` but it cannot be used to pass flags to
the operator code being tested using `--up-local` flag

This resolves operator-framework#1476 : **Custom flags for test local (no go test ones)**

Signed-off-by: Nikhil Thomas <nikthoma@redhat.com >
@nikhil-thomas nikhil-thomas force-pushed the test_--up-local_runtime-argument-flag branch from e0fbbb5 to f97341c Compare June 3, 2019 06:47
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 3, 2019
@nikhil-thomas nikhil-thomas changed the title Add --operator-flags for testing with --up-local Add --local-operator-flags for testing with --up-local Jun 3, 2019
@smiklosovic
Copy link

it would be nice to see this merged in!

@lilic lilic requested a review from AlexNPavel June 4, 2019 10:45
Copy link
Contributor

@AlexNPavel AlexNPavel left a comment

Choose a reason for hiding this comment

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

LGTM. This needs some E2E tests to make sure we don't accidentally break the functionality later, but we can handle that in a separate PR.

@nikhil-thomas
Copy link
Contributor Author

LGTM. This needs some E2E tests to make sure we don't accidentally break the functionality later, but we can handle that in a separate PR.

true, I shall add an e2e test in a different PR. 👍

Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom flags for test local (no go test ones)
5 participants