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

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)

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)
+        return self._put(idp_id, put_body)
-        self.expected_success(201, resp.status)
-        body = json.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def list_identity_providers(self):
         """List the identity providers."""
-        resp, body = self._get()
+        return self._get()
-        self.expected_success(200, resp.status)
-        body = json.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def show_identity_provider(self, idp_id):
         """Get an identity provider."""
-        resp, body = self._get(idp_id)
+        return self._get(idp_id)
-        self.expected_success(200, resp.status)
-        body = json.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def delete_identity_provider(self, idp_id):
         """Delete an identity provider."""
-        resp, body = self._delete(idp_id)
+        return self._delete(idp_id)
-        self.expected_success(204, resp.status)
-        return rest_client.ResponseBody(resp, body)
 
     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)
+        return self._patch(idp_id, patch_body)
-        self.expected_success(200, resp.status)
-        body = json.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def add_protocol_and_mapping(self, idp_id, protocol_id, mapping_id):
         """Add a protocol and mapping to an identity provider."""

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
+import json
-
-from tempest.lib.common import rest_client
 
 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})
+        put_body = json.dumps({'mapping': rules})
-        resp, body = self._put(mapping_id, put_body)
+        return self._put(mapping_id, put_body)
-        self.expected_success(201, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def list_mapping_rules(self):
         """List the mapping rules."""
-        resp, body = self._get()
+        return self._get()
-        self.expected_success(200, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def show_mapping_rule(self, mapping_id):
         """Get a mapping rule."""
-        resp, body = self._get(mapping_id)
+        return self._get(mapping_id)
-        self.expected_success(200, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def delete_mapping_rule(self, mapping_id):
         """Delete a mapping rule."""
-        resp, body = self._delete(mapping_id)
+        return self._delete(mapping_id)
-        self.expected_success(204, resp.status)
-        return rest_client.ResponseBody(resp, body)
 
     def update_mapping_rule(self, mapping_id, rules):
         """Update a mapping rule."""
-        patch_body = jsonutils.dumps({'mapping': rules})
+        patch_body = json.dumps({'mapping': rules})
-        resp, body = self._patch(mapping_id, patch_body)
+        return self._patch(mapping_id, patch_body)
-        self.expected_success(200, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)

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
+import json
-
-from tempest.lib.common import rest_client
 
 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})
+        put_body = json.dumps({'service_provider': kwargs})
-        resp, body = self._put(sp_id, put_body)
+        return self._put(sp_id, put_body)
-        self.expected_success(201, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def list_service_providers(self):
         """List the service providers."""
-        resp, body = self._get()
+        return self._get()
-        self.expected_success(200, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def show_service_provider(self, sp_id):
         """Get a service provider."""
-        resp, body = self._get(sp_id)
+        return self._get(sp_id)
-        self.expected_success(200, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def delete_service_provider(self, sp_id):
         """Delete a service provider."""
-        resp, body = self._delete(sp_id)
+        return self._delete(sp_id)
-        self.expected_success(204, resp.status)
-        return rest_client.ResponseBody(resp, body)
 
     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})
+        patch_body = json.dumps({'service_provider': kwargs})
-        resp, body = self._patch(sp_id, patch_body)
+        return self._patch(sp_id, patch_body)
-        self.expected_success(200, resp.status)
-        body = jsonutils.loads(body)
-        return rest_client.ResponseBody(resp, body)
 
     def get_service_providers_in_token(self):
         """Get the service providers list present in the token.

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)

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)

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())