Merge "Remove network debug"
diff --git a/HACKING.rst b/HACKING.rst
index 7363e7f..607682b 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -11,7 +11,7 @@
 - [T102] Cannot import OpenStack python clients in tempest/api &
          tempest/scenario tests
 - [T104] Scenario tests require a services decorator
-- [T105] Unit tests cannot use setUpClass
+- [T105] Tests cannot use setUpClass/tearDownClass
 - [T106] vim configuration should not be kept in source files.
 - [N322] Method's default argument shouldn't be mutable
 
@@ -108,6 +108,46 @@
 in tempest.api.compute would require a service tag for those services, however
 they do not need to be tagged as compute.
 
+Test fixtures and resources
+---------------------------
+Test level resources should be cleaned-up after the test execution. Clean-up
+is best scheduled using `addCleanup` which ensures that the resource cleanup
+code is always invoked, and in reverse order with respect to the creation
+order.
+
+Test class level resources should be defined in the `resource_setup` method of
+the test class, except for any credential obtained from the credentials
+provider, which should be set-up in the `setup_credentials` method.
+
+The test base class `BaseTestCase` defines Tempest framework for class level
+fixtures. `setUpClass` and `tearDownClass` are defined here and cannot be
+overwritten by subclasses (enforced via hacking rule T105).
+
+Set-up is split in a series of steps (setup stages), which can be overwritten
+by test classes. Set-up stages are:
+- `skip_checks`
+- `setup_credentials`
+- `setup_clients`
+- `resource_setup`
+
+Tear-down is also split in a series of steps (teardown stages), which are
+stacked for execution only if the corresponding setup stage had been
+reached during the setup phase. Tear-down stages are:
+- `clear_isolated_creds` (defined in the base test class)
+- `resource_cleanup`
+
+Skipping Tests
+--------------
+Skipping tests should be based on configuration only. If that is not possible,
+it is likely that either a configuration flag is missing, or the test should
+fail rather than be skipped.
+Using discovery for skipping tests is generally discouraged.
+
+When running a test that requires a certain "feature" in the target
+cloud, if that feature is missing we should fail, because either the test
+configuration is invalid, or the cloud is broken and the expected "feature" is
+not there even if the cloud was configured with it.
+
 Negative Tests
 --------------
 Newly added negative tests should use the negative test framework. First step
diff --git a/tempest/api/network/test_ports.py b/tempest/api/network/test_ports.py
index a03e587..9cd5e2e 100644
--- a/tempest/api/network/test_ports.py
+++ b/tempest/api/network/test_ports.py
@@ -66,6 +66,23 @@
         self.assertEqual(updated_port['name'], new_name)
         self.assertFalse(updated_port['admin_state_up'])
 
+    def test_create_bulk_port(self):
+        network1 = self.network
+        name = data_utils.rand_name('network-')
+        network2 = self.create_network(network_name=name)
+        network_list = [network1['id'], network2['id']]
+        port_list = [{'network_id': net_id} for net_id in network_list]
+        _, body = self.client.create_bulk_port(port_list)
+        created_ports = body['ports']
+        port1 = created_ports[0]
+        port2 = created_ports[1]
+        self.addCleanup(self._delete_port, port1['id'])
+        self.addCleanup(self._delete_port, port2['id'])
+        self.assertEqual(port1['network_id'], network1['id'])
+        self.assertEqual(port2['network_id'], network2['id'])
+        self.assertTrue(port1['admin_state_up'])
+        self.assertTrue(port2['admin_state_up'])
+
     @test.attr(type='smoke')
     def test_show_port(self):
         # Verify the details of port
diff --git a/tempest/api/network/test_routers.py b/tempest/api/network/test_routers.py
index 2b4e60a..34650c5 100644
--- a/tempest/api/network/test_routers.py
+++ b/tempest/api/network/test_routers.py
@@ -190,11 +190,12 @@
         self.assertEqual(len(list_body['ports']), 1)
         gw_port = list_body['ports'][0]
         fixed_ips = gw_port['fixed_ips']
-        self.assertEqual(len(fixed_ips), 1)
+        self.assertGreaterEqual(len(fixed_ips), 1)
         resp, public_net_body = self.admin_client.show_network(
             CONF.network.public_network_id)
         public_subnet_id = public_net_body['network']['subnets'][0]
-        self.assertEqual(fixed_ips[0]['subnet_id'], public_subnet_id)
+        self.assertIn(public_subnet_id,
+                      map(lambda x: x['subnet_id'], fixed_ips))
 
     @test.attr(type='smoke')
     def test_update_router_set_gateway(self):
diff --git a/tempest/common/rest_client.py b/tempest/common/rest_client.py
index 4c3905c..1df8896 100644
--- a/tempest/common/rest_client.py
+++ b/tempest/common/rest_client.py
@@ -33,25 +33,11 @@
 
 # redrive rate limited calls at most twice
 MAX_RECURSION_DEPTH = 2
-TOKEN_CHARS_RE = re.compile('^[-A-Za-z0-9+/=]*$')
 
 # All the successful HTTP status codes from RFC 7231 & 4918
 HTTP_SUCCESS = (200, 201, 202, 203, 204, 205, 206, 207)
 
 
-# convert a structure into a string safely
-def safe_body(body, maxlen=4096):
-    try:
-        text = six.text_type(body)
-    except UnicodeDecodeError:
-        # if this isn't actually text, return marker that
-        return "<BinaryData: removed>"
-    if len(text) > maxlen:
-        return text[:maxlen]
-    else:
-        return text
-
-
 class ResponseBody(dict):
     """Class that wraps an http response and dict body into a single value.
 
@@ -296,6 +282,18 @@
                 return resp[i]
         return ""
 
+    def _safe_body(self, body, maxlen=4096):
+        # convert a structure into a string safely
+        try:
+            text = six.text_type(body)
+        except UnicodeDecodeError:
+            # if this isn't actually text, return marker that
+            return "<BinaryData: removed>"
+        if len(text) > maxlen:
+            return text[:maxlen]
+        else:
+            return text
+
     def _log_request_start(self, method, req_url, req_headers=None,
                            req_body=None):
         if req_headers is None:
@@ -326,9 +324,9 @@
                 req_url,
                 secs,
                 str(req_headers),
-                safe_body(req_body),
+                self._safe_body(req_body),
                 str(resp),
-                safe_body(resp_body)),
+                self._safe_body(resp_body)),
             extra=extra)
 
     def _log_request(self, method, req_url, resp,
diff --git a/tempest/exceptions.py b/tempest/exceptions.py
index cc31fad..213d5de 100644
--- a/tempest/exceptions.py
+++ b/tempest/exceptions.py
@@ -75,7 +75,7 @@
     message = 'Unauthorized'
 
 
-class InvalidServiceTag(RestClientException):
+class InvalidServiceTag(TempestException):
     message = "Invalid service tag"
 
 
@@ -140,15 +140,15 @@
     message = "Endpoint not found"
 
 
-class RateLimitExceeded(TempestException):
+class RateLimitExceeded(RestClientException):
     message = "Rate limit exceeded"
 
 
-class OverLimit(TempestException):
+class OverLimit(RestClientException):
     message = "Quota exceeded"
 
 
-class ServerFault(TempestException):
+class ServerFault(RestClientException):
     message = "Got server fault"
 
 
diff --git a/tempest/scenario/test_network_basic_ops.py b/tempest/scenario/test_network_basic_ops.py
index 8f71b6e..98e3fda 100644
--- a/tempest/scenario/test_network_basic_ops.py
+++ b/tempest/scenario/test_network_basic_ops.py
@@ -396,6 +396,9 @@
         self._hotplug_server()
         self._check_network_internal_connectivity(network=self.new_net)
 
+    @testtools.skipIf(CONF.baremetal.driver_enabled,
+                      'Router state cannot be altered on a shared baremetal '
+                      'network')
     @test.attr(type='smoke')
     @test.services('compute', 'network')
     def test_update_router_admin_state(self):
diff --git a/tempest/services/network/json/network_client.py b/tempest/services/network/json/network_client.py
index 46475f0..809c98b 100644
--- a/tempest/services/network/json/network_client.py
+++ b/tempest/services/network/json/network_client.py
@@ -40,8 +40,13 @@
     def deserialize_list(self, body):
         res = json.loads(body)
         # expecting response in form
-        # {'resources': [ res1, res2] }
-        return res[res.keys()[0]]
+        # {'resources': [ res1, res2] } => when pagination disabled
+        # {'resources': [..], 'resources_links': {}} => if pagination enabled
+        pagination_suffix = "_links"
+        for k in res.keys():
+            if k[-len(pagination_suffix):] == pagination_suffix:
+                continue
+            return res[k]
 
     def serialize(self, data):
         return json.dumps(data)
diff --git a/tempest/test.py b/tempest/test.py
index 7db0376..6deb42b 100644
--- a/tempest/test.py
+++ b/tempest/test.py
@@ -224,6 +224,23 @@
 
 class BaseTestCase(testtools.testcase.WithAttributes,
                    testtools.TestCase):
+    """The test base class defines Tempest framework for class level fixtures.
+    `setUpClass` and `tearDownClass` are defined here and cannot be overwritten
+    by subclasses (enforced via hacking rule T105).
+
+    Set-up is split in a series of steps (setup stages), which can be
+    overwritten by test classes. Set-up stages are:
+    - skip_checks
+    - setup_credentials
+    - setup_clients
+    - resource_setup
+
+    Tear-down is also split in a series of steps (teardown stages), which are
+    stacked for execution only if the corresponding setup stage had been
+    reached during the setup phase. Tear-down stages are:
+    - clear_isolated_creds (defined in the base test class)
+    - resource_cleanup
+    """
 
     setUpClassCalled = False
     _service = None
@@ -242,31 +259,28 @@
         if hasattr(super(BaseTestCase, cls), 'setUpClass'):
             super(BaseTestCase, cls).setUpClass()
         cls.setUpClassCalled = True
-        # No test resource is allocated until here
+        # Stack of (name, callable) to be invoked in reverse order at teardown
+        cls.teardowns = []
+        # All the configuration checks that may generate a skip
+        cls.skip_checks()
         try:
-            # TODO(andreaf) Split-up resource_setup in stages:
-            # skip checks, pre-hook, credentials, clients, resources, post-hook
+            # Allocation of all required credentials and client managers
+            cls.teardowns.append(('credentials', cls.clear_isolated_creds))
+            cls.setup_credentials()
+            # Shortcuts to clients
+            cls.setup_clients()
+            # Additional class-wide test resources
+            cls.teardowns.append(('resources', cls.resource_cleanup))
             cls.resource_setup()
         except Exception:
             etype, value, trace = sys.exc_info()
-            LOG.info("%s in resource setup. Invoking tearDownClass." % etype)
-            # Catch any exception in tearDown so we can re-raise the original
-            # exception at the end
+            LOG.info("%s in %s.setUpClass. Invoking tearDownClass." % (
+                cls.__name__, etype))
+            cls.tearDownClass()
             try:
-                cls.tearDownClass()
-            except Exception as te:
-                tetype, _, _ = sys.exc_info()
-                # TODO(gmann): Till we split-up resource_setup &
-                # resource_cleanup in more structural way, log
-                # AttributeError as info instead of exception.
-                if tetype is AttributeError:
-                    LOG.info("tearDownClass failed: %s" % te)
-                else:
-                    LOG.exception("tearDownClass failed: %s" % te)
-            try:
-                raise etype(value), None, trace
+                raise etype, value, trace
             finally:
-                del trace  # for avoiding circular refs
+                del trace  # to avoid circular refs
 
     @classmethod
     def tearDownClass(cls):
@@ -274,21 +288,78 @@
         # It should never be overridden by descendants
         if hasattr(super(BaseTestCase, cls), 'tearDownClass'):
             super(BaseTestCase, cls).tearDownClass()
-        try:
-            cls.resource_cleanup()
-        finally:
-            cls.clear_isolated_creds()
+        # Save any existing exception, we always want to re-raise the original
+        # exception only
+        etype, value, trace = sys.exc_info()
+        # If there was no exception during setup we shall re-raise the first
+        # exception in teardown
+        re_raise = (etype is None)
+        while cls.teardowns:
+            name, teardown = cls.teardowns.pop()
+            # Catch any exception in tearDown so we can re-raise the original
+            # exception at the end
+            try:
+                teardown()
+            except Exception as te:
+                sys_exec_info = sys.exc_info()
+                tetype = sys_exec_info[0]
+                # TODO(andreaf): Till we have the ability to cleanup only
+                # resources that were successfully setup in resource_cleanup,
+                # log AttributeError as info instead of exception.
+                if tetype is AttributeError and name == 'resources':
+                    LOG.info("tearDownClass of %s failed: %s" % (name, te))
+                else:
+                    LOG.exception("teardown of %s failed: %s" % (name, te))
+                if not etype:
+                    etype, value, trace = sys_exec_info
+        # If exceptions were raised during teardown, an not before, re-raise
+        # the first one
+        if re_raise and etype is not None:
+            try:
+                raise etype, value, trace
+            finally:
+                del trace  # to avoid circular refs
 
     @classmethod
     def resource_setup(cls):
-        """Class level setup steps for test cases.
-        Recommended order: skip checks, credentials, clients, resources.
+        """Class level resource setup for test cases.
         """
         pass
 
     @classmethod
     def resource_cleanup(cls):
-        """Class level resource cleanup for test cases. """
+        """Class level resource cleanup for test cases.
+        Resource cleanup must be able to handle the case of partially setup
+        resources, in case a failure during `resource_setup` should happen.
+        """
+        pass
+
+    @classmethod
+    def skip_checks(cls):
+        """Class level skip checks. Subclasses verify in here all
+        conditions that might prevent the execution of the entire test class.
+        Checks implemented here may not make use API calls, and should rely on
+        configuration alone.
+        In general skip checks that require an API call are discouraged.
+        If one is really needed it may be implemented either in the
+        resource_setup or at test level.
+        """
+        pass
+
+    @classmethod
+    def setup_credentials(cls):
+        """Allocate credentials and the client managers from them."""
+        # TODO(andreaf) There is a fair amount of code that could me moved from
+        # base / test classes in here. Ideally tests should be able to only
+        # specify a list of (additional) credentials the need to use.
+        pass
+
+    @classmethod
+    def setup_clients(cls):
+        """Create links to the clients into the test object."""
+        # TODO(andreaf) There is a fair amount of code that could me moved from
+        # base / test classes in here. Ideally tests should be able to only
+        # specify which client is `client` and nothing else.
         pass
 
     def setUp(self):