From 76e9209801c5255bb03c36cdf372e3afb614fd57 Mon Sep 17 00:00:00 2001 From: Rodrigo Duarte Sousa Date: Thu, 16 Jun 2016 09:31:01 -0300 Subject: [PATCH] Integration tests cleanup This patch does a cleanup and fixes some nits found by reviewers in the original patches [1], some of them are: - import json instead of jsonutils - use six.moves.http_client - put common logic on clients superclass - use "fails" to indicate negative cases - stronger comparison in update tests [1] https://review.openstack.org/#/q/topic:federation_integration_tests Change-Id: I216fc5d4758e7b09d167d9d26271ddd149c66816 --- .../services/identity/clients.py | 22 ++++++++++--- .../identity/v3/identity_providers_client.py | 24 +++----------- .../identity/v3/mapping_rules_client.py | 32 +++++-------------- .../identity/v3/service_providers_client.py | 32 +++++-------------- .../identity/v3/test_identity_providers.py | 4 +++ .../api/identity/v3/test_mapping_rules.py | 7 +++- .../api/identity/v3/test_service_providers.py | 6 +++- 7 files changed, 54 insertions(+), 73 deletions(-) diff --git a/keystone_tempest_plugin/services/identity/clients.py b/keystone_tempest_plugin/services/identity/clients.py index d8c869232..caf8b527d 100644 --- a/keystone_tempest_plugin/services/identity/clients.py +++ b/keystone_tempest_plugin/services/identity/clients.py @@ -12,6 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. +import json + +from six.moves import http_client from tempest import config from tempest.lib.common import rest_client @@ -48,16 +51,27 @@ class Federation(Identity): def _delete(self, entity_id, **kwargs): url = self._build_path(entity_id) - return super(Federation, self).delete(url, **kwargs) + resp, body = super(Federation, self).delete(url, **kwargs) + self.expected_success(http_client.NO_CONTENT, resp.status) + return rest_client.ResponseBody(resp, body) def _get(self, entity_id=None, **kwargs): url = self._build_path(entity_id) - return super(Federation, self).get(url, **kwargs) + resp, body = super(Federation, self).get(url, **kwargs) + self.expected_success(http_client.OK, resp.status) + body = json.loads(body) + return rest_client.ResponseBody(resp, body) def _patch(self, entity_id, body, **kwargs): url = self._build_path(entity_id) - return super(Federation, self).patch(url, body, **kwargs) + resp, body = super(Federation, self).patch(url, body, **kwargs) + self.expected_success(http_client.OK, resp.status) + body = json.loads(body) + return rest_client.ResponseBody(resp, body) def _put(self, entity_id, body, **kwargs): url = self._build_path(entity_id) - return super(Federation, self).put(url, body, **kwargs) + resp, body = super(Federation, self).put(url, body, **kwargs) + self.expected_success(http_client.CREATED, resp.status) + body = json.loads(body) + return rest_client.ResponseBody(resp, body) diff --git a/keystone_tempest_plugin/services/identity/v3/identity_providers_client.py b/keystone_tempest_plugin/services/identity/v3/identity_providers_client.py index f5c2e4454..34f68996a 100644 --- a/keystone_tempest_plugin/services/identity/v3/identity_providers_client.py +++ b/keystone_tempest_plugin/services/identity/v3/identity_providers_client.py @@ -31,30 +31,19 @@ class IdentityProvidersClient(clients.Federation): (boolean) and remote_ids (list). """ put_body = json.dumps({'identity_provider': kwargs}) - resp, body = self._put(idp_id, put_body) - self.expected_success(201, resp.status) - body = json.loads(body) - return rest_client.ResponseBody(resp, body) + return self._put(idp_id, put_body) def list_identity_providers(self): """List the identity providers.""" - resp, body = self._get() - self.expected_success(200, resp.status) - body = json.loads(body) - return rest_client.ResponseBody(resp, body) + return self._get() def show_identity_provider(self, idp_id): """Get an identity provider.""" - resp, body = self._get(idp_id) - self.expected_success(200, resp.status) - body = json.loads(body) - return rest_client.ResponseBody(resp, body) + return self._get(idp_id) def delete_identity_provider(self, idp_id): """Delete an identity provider.""" - resp, body = self._delete(idp_id) - self.expected_success(204, resp.status) - return rest_client.ResponseBody(resp, body) + return self._delete(idp_id) def update_identity_provider(self, idp_id, **kwargs): """Update an identity provider. @@ -64,10 +53,7 @@ class IdentityProvidersClient(clients.Federation): enabled (boolean) and remote_ids (list). """ patch_body = json.dumps({'identity_provider': kwargs}) - resp, body = self._patch(idp_id, patch_body) - self.expected_success(200, resp.status) - body = json.loads(body) - return rest_client.ResponseBody(resp, body) + return self._patch(idp_id, patch_body) def add_protocol_and_mapping(self, idp_id, protocol_id, mapping_id): """Add a protocol and mapping to an identity provider.""" diff --git a/keystone_tempest_plugin/services/identity/v3/mapping_rules_client.py b/keystone_tempest_plugin/services/identity/v3/mapping_rules_client.py index 55547563e..b7fcbd72a 100644 --- a/keystone_tempest_plugin/services/identity/v3/mapping_rules_client.py +++ b/keystone_tempest_plugin/services/identity/v3/mapping_rules_client.py @@ -12,9 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_serialization import jsonutils - -from tempest.lib.common import rest_client +import json from keystone_tempest_plugin.services.identity import clients @@ -25,36 +23,22 @@ class MappingRulesClient(clients.Federation): def create_mapping_rule(self, mapping_id, rules): """Create a mapping rule.""" - put_body = jsonutils.dumps({'mapping': rules}) - resp, body = self._put(mapping_id, put_body) - self.expected_success(201, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + put_body = json.dumps({'mapping': rules}) + return self._put(mapping_id, put_body) def list_mapping_rules(self): """List the mapping rules.""" - resp, body = self._get() - self.expected_success(200, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + return self._get() def show_mapping_rule(self, mapping_id): """Get a mapping rule.""" - resp, body = self._get(mapping_id) - self.expected_success(200, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + return self._get(mapping_id) def delete_mapping_rule(self, mapping_id): """Delete a mapping rule.""" - resp, body = self._delete(mapping_id) - self.expected_success(204, resp.status) - return rest_client.ResponseBody(resp, body) + return self._delete(mapping_id) def update_mapping_rule(self, mapping_id, rules): """Update a mapping rule.""" - patch_body = jsonutils.dumps({'mapping': rules}) - resp, body = self._patch(mapping_id, patch_body) - self.expected_success(200, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + patch_body = json.dumps({'mapping': rules}) + return self._patch(mapping_id, patch_body) diff --git a/keystone_tempest_plugin/services/identity/v3/service_providers_client.py b/keystone_tempest_plugin/services/identity/v3/service_providers_client.py index 65ec9cc5f..093ce0f21 100644 --- a/keystone_tempest_plugin/services/identity/v3/service_providers_client.py +++ b/keystone_tempest_plugin/services/identity/v3/service_providers_client.py @@ -12,9 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_serialization import jsonutils - -from tempest.lib.common import rest_client +import json from keystone_tempest_plugin.services.identity import clients @@ -31,31 +29,20 @@ class ServiceProvidersClient(clients.Federation): (str). Optional: description (str), enabled (boolean) and relay_state_prefix (str). """ - put_body = jsonutils.dumps({'service_provider': kwargs}) - resp, body = self._put(sp_id, put_body) - self.expected_success(201, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + put_body = json.dumps({'service_provider': kwargs}) + return self._put(sp_id, put_body) def list_service_providers(self): """List the service providers.""" - resp, body = self._get() - self.expected_success(200, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + return self._get() def show_service_provider(self, sp_id): """Get a service provider.""" - resp, body = self._get(sp_id) - self.expected_success(200, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + return self._get(sp_id) def delete_service_provider(self, sp_id): """Delete a service provider.""" - resp, body = self._delete(sp_id) - self.expected_success(204, resp.status) - return rest_client.ResponseBody(resp, body) + return self._delete(sp_id) def update_service_provider(self, sp_id, **kwargs): """Update a service provider. @@ -65,11 +52,8 @@ class ServiceProvidersClient(clients.Federation): (str), description (str), enabled (boolean) and relay_state_prefix (str). """ - patch_body = jsonutils.dumps({'service_provider': kwargs}) - resp, body = self._patch(sp_id, patch_body) - self.expected_success(200, resp.status) - body = jsonutils.loads(body) - return rest_client.ResponseBody(resp, body) + patch_body = json.dumps({'service_provider': kwargs}) + return self._patch(sp_id, patch_body) def get_service_providers_in_token(self): """Get the service providers list present in the token. diff --git a/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py b/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py index fbc14c684..ebd00d46d 100644 --- a/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py +++ b/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py @@ -115,6 +115,10 @@ class IndentityProvidersTest(base.BaseIdentityTest): # The identity provider should be disabled self.assertFalse(idp['enabled']) + idp_get = self.idps_client.show_identity_provider( + idp_id)['identity_provider'] + self.assertFalse(idp_get['enabled']) + def _assert_protocol_attributes(self, protocol, protocol_id, mapping_id=None): self.assertIn('id', protocol) diff --git a/keystone_tempest_plugin/tests/api/identity/v3/test_mapping_rules.py b/keystone_tempest_plugin/tests/api/identity/v3/test_mapping_rules.py index f544671c2..e7ac47b38 100644 --- a/keystone_tempest_plugin/tests/api/identity/v3/test_mapping_rules.py +++ b/keystone_tempest_plugin/tests/api/identity/v3/test_mapping_rules.py @@ -50,7 +50,7 @@ class MappingRulesTest(base.BaseIdentityTest): @test.attr(type=['negative']) @decorators.idempotent_id('341dac45-ce1f-4f15-afdc-1f9a7d7d7c40') - def test_mapping_rules_create_without_mandatory_attributes(self): + def test_mapping_rules_create_without_mandatory_attributes_fails(self): mapping_id = data_utils.rand_uuid_hex() self.assertRaises( lib_exc.BadRequest, @@ -96,3 +96,8 @@ class MappingRulesTest(base.BaseIdentityTest): mapping_id, mapping_ref)['mapping'] self._assert_mapping_rules_attributes( mapping, mapping_id, mapping_ref) + + mapping_get = self.mappings_client.show_mapping_rule(mapping_id)[ + 'mapping'] + self._assert_mapping_rules_attributes( + mapping_get, mapping_id, mapping) diff --git a/keystone_tempest_plugin/tests/api/identity/v3/test_service_providers.py b/keystone_tempest_plugin/tests/api/identity/v3/test_service_providers.py index 7cecbe090..26363d4ef 100644 --- a/keystone_tempest_plugin/tests/api/identity/v3/test_service_providers.py +++ b/keystone_tempest_plugin/tests/api/identity/v3/test_service_providers.py @@ -161,9 +161,13 @@ class ServiceProvidersTest(base.BaseIdentityTest): # The service provider should be now disabled self.assertFalse(sp['enabled']) + sp_get = self.sps_client.show_service_provider(sp_id)[ + 'service_provider'] + self.assertFalse(sp_get['enabled']) + @test.attr(type=['negative']) @decorators.idempotent_id('91ce1183-1a15-4598-ae5f-85cfa98a1c77') - def test_service_provider_update_with_bad_attributes(self): + def test_service_provider_update_with_bad_attributes_fails(self): sp_id = data_utils.rand_uuid_hex() self._create_sp(sp_id, fixtures.sp_ref())