Merge "Migrate test_volume_boot_pattern to tempest client"
diff --git a/REVIEWING.rst b/REVIEWING.rst
new file mode 100644
index 0000000..d6dc83e
--- /dev/null
+++ b/REVIEWING.rst
@@ -0,0 +1,60 @@
+Reviewing Tempest Code
+======================
+
+To start read the `OpenStack Common Review Checklist
+<https://wiki.openstack.org/wiki/ReviewChecklist#Common_Review_Checklist>`_
+
+
+Ensuring code is executed
+-------------------------
+
+For any new or change to a test it has to be verified in the gate. This means
+that the first thing to check with any change is that a gate job actually runs
+it. Tests which aren't executed either because of configuration or skips should
+not be accepted.
+
+
+Unit Tests
+----------
+
+For any change that adds new functionality to either common functionality or an
+out-of-band tool unit tests are required. This is to ensure we don't introduce
+future regressions and to test conditions which we may not hit in the gate runs.
+Tests, and service clients aren't required to have unit tests since they should
+be self verifying by running them in the gate.
+
+
+API Stability
+-------------
+Tests should only be added for a published stable APIs. If a patch contains
+tests for an API which hasn't been marked as stable or for an API that which
+doesn't conform to the `API stability guidelines
+<https://wiki.openstack.org/wiki/Governance/Approved/APIStability>`_ then it
+should not be approved.
+
+
+Reject Copy and Paste Test Code
+------------------------
+When creating new tests that are similar to existing tests it is tempting to
+simply copy the code and make a few modifications. This increases code size and
+the maintenance burden. Such changes should not be approved if it is easy to
+abstract the duplicated code into a function or method.
+
+
+Being explicit
+--------------
+When tests are being added that depend on a configurable feature or extension,
+polling the API to discover that it is enabled should not be done. This will
+just result in bugs being masked because the test can be skipped automatically.
+Instead the config file should be used to determine whether a test should be
+skipped or not. Do not approve changes that depend on an API call to determine
+whether to skip or not.
+
+
+When to approve
+---------------
+ * Every patch needs two +2s before being approved.
+ * Its ok to hold off on an approval until a subject matter expert reviews it
+ * If a patch has already been approved but requires a trivial rebase to merge,
+   you do not have to wait for a second +2, since the patch has already had
+   two +2s.
diff --git a/doc/source/REVIEWING.rst b/doc/source/REVIEWING.rst
new file mode 120000
index 0000000..841e042
--- /dev/null
+++ b/doc/source/REVIEWING.rst
@@ -0,0 +1 @@
+../../REVIEWING.rst
\ No newline at end of file
diff --git a/doc/source/index.rst b/doc/source/index.rst
index 25bc900..d3118ac 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -9,6 +9,7 @@
 
    overview
    HACKING
+   REVIEWING
 
 ------------
 Field Guides
diff --git a/etc/tempest.conf.sample b/etc/tempest.conf.sample
index 29f80bd..ef56ab3 100644
--- a/etc/tempest.conf.sample
+++ b/etc/tempest.conf.sample
@@ -1010,9 +1010,9 @@
 # value)
 #trove=false
 
-# Whether or not Marconi is expected to be available (boolean
+# Whether or not Zaqar is expected to be available (boolean
 # value)
-#marconi=false
+#zaqar=false
 
 
 [stress]
diff --git a/tempest/api/network/test_networks.py b/tempest/api/network/test_networks.py
index 206f37b..f3da614 100644
--- a/tempest/api/network/test_networks.py
+++ b/tempest/api/network/test_networks.py
@@ -374,7 +374,8 @@
 
     @test.attr(type='smoke')
     def test_create_delete_subnet_with_gw(self):
-        gateway = '2003::2'
+        net = netaddr.IPNetwork(CONF.network.tenant_network_v6_cidr)
+        gateway = str(netaddr.IPAddress(net.first + 2))
         name = data_utils.rand_name('network-')
         _, body = self.client.create_network(name=name)
         network = body['network']
@@ -388,13 +389,15 @@
 
     @test.attr(type='smoke')
     def test_create_delete_subnet_without_gw(self):
+        net = netaddr.IPNetwork(CONF.network.tenant_network_v6_cidr)
+        gateway_ip = str(netaddr.IPAddress(net.first + 1))
         name = data_utils.rand_name('network-')
         _, body = self.client.create_network(name=name)
         network = body['network']
         net_id = network['id']
         subnet = self.create_subnet(network)
         # Verifies Subnet GW in IPv6
-        self.assertEqual(subnet['gateway_ip'], '2003::1')
+        self.assertEqual(subnet['gateway_ip'], gateway_ip)
         # Delete network and subnet
         _, body = self.client.delete_network(net_id)
         self.subnets.pop()
diff --git a/tempest/api/queuing/base.py b/tempest/api/queuing/base.py
index f4ff7f1..41a02f2 100644
--- a/tempest/api/queuing/base.py
+++ b/tempest/api/queuing/base.py
@@ -26,7 +26,7 @@
 class BaseQueuingTest(test.BaseTestCase):
 
     """
-    Base class for the Queuing tests that use the Tempest Marconi REST client
+    Base class for the Queuing tests that use the Tempest Zaqar REST client
 
     It is assumed that the following option is defined in the
     [service_available] section of etc/tempest.conf
@@ -37,8 +37,8 @@
     @classmethod
     def setUpClass(cls):
         super(BaseQueuingTest, cls).setUpClass()
-        if not CONF.service_available.marconi:
-            raise cls.skipException("Marconi support is required")
+        if not CONF.service_available.zaqar:
+            raise cls.skipException("Zaqar support is required")
         os = cls.get_client_manager()
         cls.queuing_cfg = CONF.queuing
         cls.client = os.queuing_client
diff --git a/tempest/cmd/verify_tempest_config.py b/tempest/cmd/verify_tempest_config.py
index 70fd27b..cd696a9 100755
--- a/tempest/cmd/verify_tempest_config.py
+++ b/tempest/cmd/verify_tempest_config.py
@@ -247,7 +247,7 @@
         'data_processing': 'sahara',
         'baremetal': 'ironic',
         'identity': 'keystone',
-        'queuing': 'marconi',
+        'queuing': 'zaqar',
         'database': 'trove'
     }
     # Get catalog list for endpoints to use for validation
diff --git a/tempest/common/generator/base_generator.py b/tempest/common/generator/base_generator.py
index 57b98f7..0398af1 100644
--- a/tempest/common/generator/base_generator.py
+++ b/tempest/common/generator/base_generator.py
@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import functools
+
 import jsonschema
 
 from tempest.openstack.common import log as logging
@@ -39,6 +41,7 @@
     """
     Decorator for simple generators that return one value
     """
+    @functools.wraps(fn)
     def wrapped(self, schema):
         result = fn(self, schema)
         if result is not None:
diff --git a/tempest/common/isolated_creds.py b/tempest/common/isolated_creds.py
index d5e49db..dca1f86 100644
--- a/tempest/common/isolated_creds.py
+++ b/tempest/common/isolated_creds.py
@@ -13,6 +13,7 @@
 #    under the License.
 
 import netaddr
+from neutronclient.common import exceptions as n_exc
 
 from tempest import auth
 from tempest import clients
@@ -263,7 +264,7 @@
                     body['subnet']['cidr'] = str(subnet_cidr)
                     resp_body = self.network_admin_client.create_subnet(body)
                 break
-            except exceptions.BadRequest as e:
+            except (n_exc.BadRequest, exceptions.BadRequest) as e:
                 if 'overlaps with another subnet' not in str(e):
                     raise
         else:
diff --git a/tempest/config.py b/tempest/config.py
index af45ba5..1d10a0a 100644
--- a/tempest/config.py
+++ b/tempest/config.py
@@ -921,9 +921,9 @@
     cfg.BoolOpt('trove',
                 default=False,
                 help="Whether or not Trove is expected to be available"),
-    cfg.BoolOpt('marconi',
+    cfg.BoolOpt('zaqar',
                 default=False,
-                help="Whether or not Marconi is expected to be available"),
+                help="Whether or not Zaqar is expected to be available"),
 ]
 
 debug_group = cfg.OptGroup(name="debug",
diff --git a/tempest/hacking/checks.py b/tempest/hacking/checks.py
index 93329bc..cef010e 100644
--- a/tempest/hacking/checks.py
+++ b/tempest/hacking/checks.py
@@ -20,7 +20,7 @@
 
 PYTHON_CLIENTS = ['cinder', 'glance', 'keystone', 'nova', 'swift', 'neutron',
                   'trove', 'ironic', 'savanna', 'heat', 'ceilometer',
-                  'marconi', 'sahara']
+                  'zaqar', 'sahara']
 
 PYTHON_CLIENT_RE = re.compile('import (%s)client' % '|'.join(PYTHON_CLIENTS))
 TEST_DEFINITION = re.compile(r'^\s*def test.*')
diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py
index d1af4bc..0f14c94 100644
--- a/tempest/scenario/manager.py
+++ b/tempest/scenario/manager.py
@@ -1814,3 +1814,81 @@
             self.client.stacks.delete(stack_identifier)
         except heat_exceptions.HTTPNotFound:
             pass
+
+
+class SwiftScenarioTest(ScenarioTest):
+    """
+    Provide harness to do Swift scenario tests.
+
+    Subclasses implement the tests that use the methods provided by this
+    class.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        cls.set_network_resources()
+        super(SwiftScenarioTest, cls).setUpClass()
+        if not CONF.service_available.swift:
+            skip_msg = ("%s skipped as swift is not available" %
+                        cls.__name__)
+            raise cls.skipException(skip_msg)
+        # Clients for Swift
+        cls.account_client = cls.manager.account_client
+        cls.container_client = cls.manager.container_client
+        cls.object_client = cls.manager.object_client
+
+    def _get_swift_stat(self):
+        """get swift status for our user account."""
+        self.account_client.list_account_containers()
+        LOG.debug('Swift status information obtained successfully')
+
+    def _create_container(self, container_name=None):
+        name = container_name or data_utils.rand_name(
+            'swift-scenario-container')
+        self.container_client.create_container(name)
+        # look for the container to assure it is created
+        self._list_and_check_container_objects(name)
+        LOG.debug('Container %s created' % (name))
+        return name
+
+    def _delete_container(self, container_name):
+        self.container_client.delete_container(container_name)
+        LOG.debug('Container %s deleted' % (container_name))
+
+    def _upload_object_to_container(self, container_name, obj_name=None):
+        obj_name = obj_name or data_utils.rand_name('swift-scenario-object')
+        obj_data = data_utils.arbitrary_string()
+        self.object_client.create_object(container_name, obj_name, obj_data)
+        return obj_name, obj_data
+
+    def _delete_object(self, container_name, filename):
+        self.object_client.delete_object(container_name, filename)
+        self._list_and_check_container_objects(container_name,
+                                               not_present_obj=[filename])
+
+    def _list_and_check_container_objects(self, container_name, present_obj=[],
+                                          not_present_obj=[]):
+        """
+        List objects for a given container and assert which are present and
+        which are not.
+        """
+        _, object_list = self.container_client.list_container_contents(
+            container_name)
+        if present_obj:
+            for obj in present_obj:
+                self.assertIn(obj, object_list)
+        if not_present_obj:
+            for obj in not_present_obj:
+                self.assertNotIn(obj, object_list)
+
+    def _change_container_acl(self, container_name, acl):
+        metadata_param = {'metadata_prefix': 'x-container-',
+                          'metadata': {'read': acl}}
+        self.container_client.update_container_metadata(container_name,
+                                                        **metadata_param)
+        resp, _ = self.container_client.list_container_metadata(container_name)
+        self.assertEqual(resp['x-container-read'], acl)
+
+    def _download_and_verify(self, container_name, obj_name, expected_data):
+        _, obj = self.object_client.get_object(container_name, obj_name)
+        self.assertEqual(obj, expected_data)
diff --git a/tempest/scenario/test_swift_basic_ops.py b/tempest/scenario/test_swift_basic_ops.py
index 3fa6d2c..ad74ec4 100644
--- a/tempest/scenario/test_swift_basic_ops.py
+++ b/tempest/scenario/test_swift_basic_ops.py
@@ -14,7 +14,6 @@
 #    under the License.
 
 from tempest.common import http
-from tempest.common.utils import data_utils
 from tempest import config
 from tempest.openstack.common import log as logging
 from tempest.scenario import manager
@@ -25,9 +24,9 @@
 LOG = logging.getLogger(__name__)
 
 
-class TestSwiftBasicOps(manager.ScenarioTest):
+class TestSwiftBasicOps(manager.SwiftScenarioTest):
     """
-    Test swift with the follow operations:
+    Test swift basic ops.
      * get swift stat.
      * create container.
      * upload a file to the created container.
@@ -40,75 +39,6 @@
      * change ACL of the container and make sure it works successfully
     """
 
-    @classmethod
-    def setUpClass(cls):
-        cls.set_network_resources()
-        super(TestSwiftBasicOps, cls).setUpClass()
-        if not CONF.service_available.swift:
-            skip_msg = ("%s skipped as swift is not available" %
-                        cls.__name__)
-            raise cls.skipException(skip_msg)
-        # Clients for Swift
-        cls.account_client = cls.manager.account_client
-        cls.container_client = cls.manager.container_client
-        cls.object_client = cls.manager.object_client
-
-    def _get_swift_stat(self):
-        """get swift status for our user account."""
-        self.account_client.list_account_containers()
-        LOG.debug('Swift status information obtained successfully')
-
-    def _create_container(self, container_name=None):
-        name = container_name or data_utils.rand_name(
-            'swift-scenario-container')
-        self.container_client.create_container(name)
-        # look for the container to assure it is created
-        self._list_and_check_container_objects(name)
-        LOG.debug('Container %s created' % (name))
-        return name
-
-    def _delete_container(self, container_name):
-        self.container_client.delete_container(container_name)
-        LOG.debug('Container %s deleted' % (container_name))
-
-    def _upload_object_to_container(self, container_name, obj_name=None):
-        obj_name = obj_name or data_utils.rand_name('swift-scenario-object')
-        obj_data = data_utils.arbitrary_string()
-        self.object_client.create_object(container_name, obj_name, obj_data)
-        return obj_name, obj_data
-
-    def _delete_object(self, container_name, filename):
-        self.object_client.delete_object(container_name, filename)
-        self._list_and_check_container_objects(container_name,
-                                               not_present_obj=[filename])
-
-    def _list_and_check_container_objects(self, container_name, present_obj=[],
-                                          not_present_obj=[]):
-        """
-        List objects for a given container and assert which are present and
-        which are not.
-        """
-        _, object_list = self.container_client.list_container_contents(
-            container_name)
-        if present_obj:
-            for obj in present_obj:
-                self.assertIn(obj, object_list)
-        if not_present_obj:
-            for obj in not_present_obj:
-                self.assertNotIn(obj, object_list)
-
-    def _change_container_acl(self, container_name, acl):
-        metadata_param = {'metadata_prefix': 'x-container-',
-                          'metadata': {'read': acl}}
-        self.container_client.update_container_metadata(container_name,
-                                                        **metadata_param)
-        resp, _ = self.container_client.list_container_metadata(container_name)
-        self.assertEqual(resp['x-container-read'], acl)
-
-    def _download_and_verify(self, container_name, obj_name, expected_data):
-        _, obj = self.object_client.get_object(container_name, obj_name)
-        self.assertEqual(obj, expected_data)
-
     @test.services('object_storage')
     def test_swift_basic_ops(self):
         self._get_swift_stat()
diff --git a/tempest/test.py b/tempest/test.py
index 59da2f9..f34933e 100644
--- a/tempest/test.py
+++ b/tempest/test.py
@@ -70,7 +70,7 @@
     """A decorator used to wrap the setUpClass for cleaning up resources
        when setUpClass failed.
     """
-
+    @functools.wraps(f)
     def decorator(cls):
             try:
                 f(cls)
@@ -399,25 +399,6 @@
         cls.admin_client = os_admin.negative_client
 
     @staticmethod
-    def load_schema(file_or_dict):
-        """
-        Loads a schema from a file_or_dict on a specified location.
-
-        :param file_or_dict: just a dict or filename
-        """
-        # NOTE(mkoderer): we will get rid of this function when all test are
-        # ported to dicts
-        if isinstance(file_or_dict, dict):
-            return file_or_dict
-
-        # NOTE(mkoderer): must be extended for xml support
-        fn = os.path.join(
-            os.path.abspath(os.path.dirname(os.path.dirname(__file__))),
-            "etc", "schemas", file_or_dict)
-        LOG.debug("Open schema file: %s" % (fn))
-        return json.load(open(fn))
-
-    @staticmethod
     def load_tests(*args):
         """
         Wrapper for testscenarios to set the mandatory scenarios variable
@@ -460,7 +441,6 @@
                 the data is used to generate query strings appended to the url,
                 otherwise for the body of the http call.
         """
-        description = NegativeAutoTest.load_schema(description)
         LOG.debug(description)
         generator = importutils.import_class(
             CONF.negative.test_generator)()
@@ -514,7 +494,6 @@
                 otherwise for the body of the http call.
 
         """
-        description = NegativeAutoTest.load_schema(description)
         LOG.info("Executing %s" % description["name"])
         LOG.debug(description)
         method = description["http-method"]
@@ -604,8 +583,6 @@
     """
     @attr(type=['negative', 'gate'])
     def generic_test(self):
-        if hasattr(self, '_schema_file'):
-            self.execute(self._schema_file)
         if hasattr(self, '_schema'):
             self.execute(self._schema)
 
diff --git a/tempest/tests/negative/test_negative_auto_test.py b/tempest/tests/negative/test_negative_auto_test.py
index edff3a8..dddd083 100644
--- a/tempest/tests/negative/test_negative_auto_test.py
+++ b/tempest/tests/negative/test_negative_auto_test.py
@@ -13,10 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import json
-
-import mock
-
 from tempest import config
 import tempest.test as test
 from tempest.tests import base
@@ -58,11 +54,9 @@
         for entry in entries:
             self.assertIsNotNone(entry[1]['resource'])
 
-    @mock.patch('tempest.test.NegativeAutoTest.load_schema')
-    def test_generate_scenario(self, open_mock):
-        open_mock.return_value = self.fake_input_desc
+    def test_generate_scenario(self):
         scenarios = test.NegativeAutoTest.\
-            generate_scenario(None)
+            generate_scenario(self.fake_input_desc)
 
         self.assertIsInstance(scenarios, list)
         for scenario in scenarios:
@@ -72,13 +66,3 @@
         self._check_prop_entries(scenarios, "prop_minRam")
         self._check_prop_entries(scenarios, "prop_minDisk")
         self._check_resource_entries(scenarios, "inv_res")
-
-    def test_load_schema(self):
-        json_schema = json.dumps(self.fake_input_desc)
-        with mock.patch('tempest.test.open',
-                        mock.mock_open(read_data=json_schema),
-                        create=True):
-            return_file = test.NegativeAutoTest.load_schema('filename')
-            self.assertEqual(return_file, self.fake_input_desc)
-        return_dict = test.NegativeAutoTest.load_schema(self.fake_input_desc)
-        self.assertEqual(return_file, return_dict)
diff --git a/tempest/tests/test_decorators.py b/tempest/tests/test_decorators.py
index 6b678f7..12104ec 100644
--- a/tempest/tests/test_decorators.py
+++ b/tempest/tests/test_decorators.py
@@ -237,7 +237,7 @@
 class TestSimpleNegativeDecorator(BaseDecoratorsTest):
     @test.SimpleNegativeAutoTest
     class FakeNegativeJSONTest(test.NegativeAutoTest):
-        _schema_file = 'fake/schemas/file.json'
+        _schema = {}
 
     def test_testfunc_exist(self):
         self.assertIn("test_fake_negative", dir(self.FakeNegativeJSONTest))
@@ -247,4 +247,4 @@
         obj = self.FakeNegativeJSONTest("test_fake_negative")
         self.assertIn("test_fake_negative", dir(obj))
         obj.test_fake_negative()
-        mock.assert_called_once_with(self.FakeNegativeJSONTest._schema_file)
+        mock.assert_called_once_with(self.FakeNegativeJSONTest._schema)