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

test validate service broker api version function #345

Merged
merged 2 commits into from
May 9, 2018

Conversation

ericavonb
Copy link
Contributor

@ericavonb ericavonb commented May 8, 2018

Add test of the implementation to validate supported versions of the Open Service Broker as per the service broker interface. All current versions (2.11, 2.12, and 2.13) are supported.

@ericavonb ericavonb requested a review from alecmerdler May 8, 2018 17:51
err := mockALMBroker(ctrl).ValidateBrokerAPIVersion("n/a")
require.EqualError(t, err, "not implemented")
// test bad version
brokerMock := mockALMBroker(ctrl)
Copy link
Member

Choose a reason for hiding this comment

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

Can these be done without the operatorclient mocks? It's just more work to remove operator client if we use them in new code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

nope, that's good - I just saw mock and assumed it was using the MockOperatorClient!

@ericavonb ericavonb force-pushed the ALM-538/listcatalog branch from d9dd9c9 to d72a161 Compare May 8, 2018 18:42
@ericavonb ericavonb changed the title implement validate service broker api version function test validate service broker api version function May 8, 2018
@ericavonb ericavonb force-pushed the ALM-538/listcatalog branch from d72a161 to 09c9908 Compare May 8, 2018 18:43
Copy link
Member

@ecordell ecordell left a comment

Choose a reason for hiding this comment

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

LGTM

testNamespace = "testns"
)

func mockALMBroker(ctrl *gomock.Controller) *ALMBroker {
Copy link
Member

Choose a reason for hiding this comment

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

you can remove ctrl, it's unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

func TestValidateBrokerAPIVersion(t *testing.T) {
ctrl := gomock.NewController(t)
Copy link
Member

Choose a reason for hiding this comment

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

same here, you don't need ctrl or the defer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericavonb ericavonb force-pushed the ALM-538/listcatalog branch from fcd9672 to f2f9847 Compare May 8, 2018 19:30
Copy link
Member

@ecordell ecordell left a comment

Choose a reason for hiding this comment

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

LGTM

@ericavonb ericavonb merged commit 2c99935 into operator-framework:master May 9, 2018
njhale pushed a commit to njhale/operator-lifecycle-manager that referenced this pull request Sep 10, 2018
…catalog

test validate service broker api version function
ecordell pushed a commit to ecordell/operator-lifecycle-manager that referenced this pull request Mar 8, 2019
…catalog

test validate service broker api version function
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