Skip to content

Conversation

@Cristi1324
Copy link
Contributor

This PR adds unit tests for coriolisclient.v1.common...endpoints modules.

Copy link
Contributor

@Dany9966 Dany9966 left a comment

Choose a reason for hiding this comment

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

Mostly unnecessary mocks. Please fix

Comment on lines 29 to 30
@mock.patch.object(common, 'encode_base64_param')
@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

These don't need mocking. I'd prefer real test examples please

@mock.patch.object(endpoint_destination_options.
EndpointDestinationOptionsManager, '_list')
@mock.patch.object(common, 'encode_base64_param')
@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

@mock.patch.object(endpoint_instances.EndpointInstanceManager, '_list')
@mock.patch.object(urlparse, 'urlencode')
@mock.patch.object(common, 'encode_base64_param')
@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.


@mock.patch.object(endpoint_instances.EndpointInstanceManager, '_get')
@mock.patch.object(common, 'encode_base64_param')
@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.


@mock.patch.object(endpoint_networks.EndpointNetworkManager, '_list')
@mock.patch.object(common, 'encode_base64_param')
@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

mock_post.assert_called_once_with(
'/endpoints', expected_data, 'endpoint')

@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

mock_put.assert_called_once_with(
'/endpoints/sentinel.id', expected_data, 'endpoint')

@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

)
mock_delete.assert_called_once_with('/endpoints/sentinel.id')

@mock.patch.object(base, 'getid')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Comment on lines 172 to 173
@mock.patch.object(endpoints.EndpointManager, '_get_endpoint_id_for_name')
@mock.patch.object(utils, 'validate_uuid_string')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

)

@mock.patch.object(endpoints.EndpointManager, 'list')
@mock.patch.object(utils, 'validate_uuid_string')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Comment on lines 36 to 37
{"env": "mock_env"},
["option1", "option2"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add keyword arg names

Suggested change
{"env": "mock_env"},
["option1", "option2"]
environment={"env": "mock_env"},
option_names=["option1", "option2"]

Comment on lines 32 to 33
{"env": "mock_env"},
["option1", "option2"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

result = self.endpoint.get(
mock_endpoint,
"mock_id",
{"env": "mock_env"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Suggested change
{"env": "mock_env"},
env={"env": "mock_env"},

ValueError,
self.endpoint.get,
mock_endpoint,
"mock_env",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"mock_env",
"mock_instance_id",


result = self.endpoint.list(
mock_endpoint,
{"env": "mock_env"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Comment on lines 36 to 37
{"env": "mock_env"},
["option1", "option2"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Comment on lines 32 to 33
{"env": "mock_env"},
["option1", "option2"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above


result = self.endpoint.list(
mock_endpoint,
{"env": "mock_env"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above


result = self.endpoint.get_default(
mock_endpoint,
{"env": "mock_env"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

}
)

def test_flavor_name(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

:)

Suggested change
def test_flavor_name(self):
def test_connection_info(self):

@Dany9966 Dany9966 merged commit f9687ba into cloudbase:master Nov 28, 2024
4 checks passed
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