Merge "Trivial: Fix some document comments in microversion"
diff --git a/tempest/api/network/test_networks.py b/tempest/api/network/test_networks.py
index d916891..fa1ed6a 100644
--- a/tempest/api/network/test_networks.py
+++ b/tempest/api/network/test_networks.py
@@ -12,10 +12,9 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-import itertools
-
 import netaddr
 import six
+import testtools
 
 from tempest.api.network import base
 from tempest.common import custom_matchers
@@ -376,6 +375,9 @@
 
     @test.attr(type='smoke')
     @test.idempotent_id('af774677-42a9-4e4b-bb58-16fe6a5bc1ec')
+    @test.requires_ext(extension='external-net', service='network')
+    @testtools.skipUnless(CONF.network.public_network_id,
+                          'The public_network_id option must be specified.')
     def test_external_network_visibility(self):
         """Verifies user can see external networks but not subnets."""
         body = self.networks_client.list_networks(**{'router:external': True})
@@ -387,17 +389,12 @@
         self.assertEmpty(nonexternal, "Found non-external networks"
                                       " in filtered list (%s)." % nonexternal)
         self.assertIn(CONF.network.public_network_id, networks)
-
-        subnets_iter = (network['subnets']
-                        for network in body['networks']
-                        if not network['shared'])
-        # subnets_iter is a list (iterator) of lists. This flattens it to a
-        # list of UUIDs
-        public_subnets_iter = itertools.chain(*subnets_iter)
-        body = self.subnets_client.list_subnets()
-        subnets = [sub['id'] for sub in body['subnets']
-                   if sub['id'] in public_subnets_iter]
-        self.assertEmpty(subnets, "Public subnets visible")
+        # only check the public network ID because the other networks may
+        # belong to other tests and their state may have changed during this
+        # test
+        body = self.subnets_client.list_subnets(
+            network_id=CONF.network.public_network_id)
+        self.assertEmpty(body['subnets'], "Public subnets visible")
 
 
 class BulkNetworkOpsTestJSON(base.BaseNetworkTest):
diff --git a/tempest/cmd/cleanup.py b/tempest/cmd/cleanup.py
index 5a52043..3706b54 100644
--- a/tempest/cmd/cleanup.py
+++ b/tempest/cmd/cleanup.py
@@ -117,7 +117,6 @@
 
         if is_dry_run:
             self.dry_run_data["_tenants_to_clean"] = {}
-            f = open(DRY_RUN_JSON, 'w+')
 
         admin_mgr = self.admin_mgr
         # Always cleanup tempest and alt tempest tenants unless
@@ -146,9 +145,9 @@
             svc.run()
 
         if is_dry_run:
-            f.write(json.dumps(self.dry_run_data, sort_keys=True,
-                               indent=2, separators=(',', ': ')))
-            f.close()
+            with open(DRY_RUN_JSON, 'w+') as f:
+                f.write(json.dumps(self.dry_run_data, sort_keys=True,
+                                   indent=2, separators=(',', ': ')))
 
         self._remove_admin_user_roles()
 
@@ -281,16 +280,15 @@
             svc = service(admin_mgr, **kwargs)
             svc.run()
 
-        f = open(SAVED_STATE_JSON, 'w+')
-        f.write(json.dumps(data,
-                           sort_keys=True, indent=2, separators=(',', ': ')))
-        f.close()
+        with open(SAVED_STATE_JSON, 'w+') as f:
+            f.write(json.dumps(data,
+                    sort_keys=True, indent=2, separators=(',', ': ')))
 
     def _load_json(self):
         try:
-            json_file = open(SAVED_STATE_JSON)
-            self.json_data = json.load(json_file)
-            json_file.close()
+            with open(SAVED_STATE_JSON) as json_file:
+                self.json_data = json.load(json_file)
+
         except IOError as ex:
             LOG.exception("Failed loading saved state, please be sure you"
                           " have first run cleanup with --init-saved-state "
diff --git a/tempest/cmd/verify_tempest_config.py b/tempest/cmd/verify_tempest_config.py
index 5e5e127..9049886 100644
--- a/tempest/cmd/verify_tempest_config.py
+++ b/tempest/cmd/verify_tempest_config.py
@@ -354,8 +354,6 @@
     outfile = sys.stdout
     if update:
         conf_file = _get_config_file()
-        if opts.output:
-            outfile = open(opts.output, 'w+')
         CONF_PARSER = moves.configparser.SafeConfigParser()
         CONF_PARSER.optionxform = str
         CONF_PARSER.readfp(conf_file)
@@ -378,8 +376,9 @@
         display_results(results, update, replace)
         if update:
             conf_file.close()
-            CONF_PARSER.write(outfile)
-        outfile.close()
+            if opts.output:
+                with open(opts.output, 'w+') as outfile:
+                    CONF_PARSER.write(outfile)
     finally:
         icreds.clear_creds()
 
diff --git a/tempest/common/compute.py b/tempest/common/compute.py
index 2fbd1b2..2d2909a 100644
--- a/tempest/common/compute.py
+++ b/tempest/common/compute.py
@@ -49,10 +49,6 @@
 
     # TODO(jlanoux) add support of wait_until PINGABLE/SSHABLE
 
-    name = name
-    flavor = flavor
-    image_id = image_id
-
     if name is None:
         name = data_utils.rand_name(__name__ + "-instance")
     if flavor is None:
@@ -152,14 +148,12 @@
 
             except Exception:
                 with excutils.save_and_reraise_exception():
-                    if ('preserve_server_on_error' not in kwargs
-                        or kwargs['preserve_server_on_error'] is False):
-                        for server in servers:
-                            try:
-                                clients.servers_client.delete_server(
-                                    server['id'])
-                            except Exception:
-                                LOG.exception('Deleting server %s failed'
-                                              % server['id'])
+                    for server in servers:
+                        try:
+                            clients.servers_client.delete_server(
+                                server['id'])
+                        except Exception:
+                            LOG.exception('Deleting server %s failed'
+                                          % server['id'])
 
     return body, servers
diff --git a/tempest/lib/cmd/skip_tracker.py b/tempest/lib/cmd/skip_tracker.py
index b5c9b95..b7d6a24 100755
--- a/tempest/lib/cmd/skip_tracker.py
+++ b/tempest/lib/cmd/skip_tracker.py
@@ -81,21 +81,23 @@
     DEF_RE = re.compile(r'\s*def (\w+)\(')
     bug_found = False
     results = []
-    lines = open(path, 'rb').readlines()
-    for x, line in enumerate(lines):
-        if not bug_found:
-            res = BUG_RE.match(line)
-            if res:
-                bug_no = int(res.group(1))
-                debug("Found bug skip %s on line %d", bug_no, x + 1)
-                bug_found = True
-        else:
-            res = DEF_RE.match(line)
-            if res:
-                method = res.group(1)
-                debug("Found test method %s skips for bug %d", method, bug_no)
-                results.append((method, bug_no))
-                bug_found = False
+    with open(path, 'rb') as content:
+        lines = content.readlines()
+        for x, line in enumerate(lines):
+            if not bug_found:
+                res = BUG_RE.match(line)
+                if res:
+                    bug_no = int(res.group(1))
+                    debug("Found bug skip %s on line %d", bug_no, x + 1)
+                    bug_found = True
+            else:
+                res = DEF_RE.match(line)
+                if res:
+                    method = res.group(1)
+                    debug("Found test method %s skips for bug %d",
+                          method, bug_no)
+                    results.append((method, bug_no))
+                    bug_found = False
     return results
 
 
diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py
index 0c16056..8ba5f9a 100644
--- a/tempest/scenario/manager.py
+++ b/tempest/scenario/manager.py
@@ -262,7 +262,7 @@
         return server
 
     def create_volume(self, size=None, name=None, snapshot_id=None,
-                      imageRef=None, volume_type=None, wait_on_delete=True):
+                      imageRef=None, volume_type=None):
         if name is None:
             name = data_utils.rand_name(self.__class__.__name__)
         kwargs = {'display_name': name,
@@ -273,17 +273,10 @@
             kwargs.update({'size': size})
         volume = self.volumes_client.create_volume(**kwargs)['volume']
 
-        if wait_on_delete:
-            self.addCleanup(self.volumes_client.wait_for_resource_deletion,
-                            volume['id'])
-            self.addCleanup(self.delete_wrapper,
-                            self.volumes_client.delete_volume, volume['id'])
-        else:
-            self.addCleanup_with_wait(
-                waiter_callable=self.volumes_client.wait_for_resource_deletion,
-                thing_id=volume['id'], thing_id_param='id',
-                cleanup_callable=self.delete_wrapper,
-                cleanup_args=[self.volumes_client.delete_volume, volume['id']])
+        self.addCleanup(self.volumes_client.wait_for_resource_deletion,
+                        volume['id'])
+        self.addCleanup(self.delete_wrapper,
+                        self.volumes_client.delete_volume, volume['id'])
 
         # NOTE(e0ne): Cinder API v2 uses name instead of display_name
         if 'display_name' in volume:
@@ -394,8 +387,6 @@
         if properties is None:
             properties = {}
         name = data_utils.rand_name('%s-' % name)
-        image_file = open(path, 'rb')
-        self.addCleanup(image_file.close)
         params = {
             'name': name,
             'container_format': fmt,
@@ -406,7 +397,8 @@
         image = self.image_client.create_image(**params)['image']
         self.addCleanup(self.image_client.delete_image, image['id'])
         self.assertEqual("queued", image['status'])
-        self.image_client.update_image(image['id'], data=image_file)
+        with open(path, 'rb') as image_file:
+            self.image_client.update_image(image['id'], data=image_file)
         return image['id']
 
     def glance_image_create(self):
diff --git a/tempest/tests/cmd/test_tempest_init.py b/tempest/tests/cmd/test_tempest_init.py
index 1048a52..685a0b3 100644
--- a/tempest/tests/cmd/test_tempest_init.py
+++ b/tempest/tests/cmd/test_tempest_init.py
@@ -36,9 +36,8 @@
         testr_conf_file = init.TESTR_CONF % (top_level_path, discover_path)
 
         conf_path = conf_dir.join('.testr.conf')
-        conf_file = open(conf_path, 'r')
-        self.addCleanup(conf_file.close)
-        self.assertEqual(conf_file.read(), testr_conf_file)
+        with open(conf_path, 'r') as conf_file:
+            self.assertEqual(conf_file.read(), testr_conf_file)
 
     def test_generate_sample_config(self):
         local_dir = self.useFixture(fixtures.TempDir())
diff --git a/tempest/tests/lib/test_decorators.py b/tempest/tests/lib/test_decorators.py
index 558445d..07b577c 100644
--- a/tempest/tests/lib/test_decorators.py
+++ b/tempest/tests/lib/test_decorators.py
@@ -109,7 +109,7 @@
         t = TestFoo('test_foo')
         if expected_to_skip:
             self.assertRaises(testtools.TestCase.skipException,
-                              t.test_foo())
+                              t.test_foo)
         else:
             try:
                 t.test_foo()
diff --git a/tools/skip_tracker.py b/tools/skip_tracker.py
index a47e217..b554514 100755
--- a/tools/skip_tracker.py
+++ b/tools/skip_tracker.py
@@ -73,21 +73,23 @@
     DEF_RE = re.compile(r'\s*def (\w+)\(')
     bug_found = False
     results = []
-    lines = open(path, 'rb').readlines()
-    for x, line in enumerate(lines):
-        if not bug_found:
-            res = BUG_RE.match(line)
-            if res:
-                bug_no = int(res.group(1))
-                debug("Found bug skip %s on line %d", bug_no, x + 1)
-                bug_found = True
-        else:
-            res = DEF_RE.match(line)
-            if res:
-                method = res.group(1)
-                debug("Found test method %s skips for bug %d", method, bug_no)
-                results.append((method, bug_no))
-                bug_found = False
+    with open(path, 'rb') as content:
+        lines = content.readlines()
+        for x, line in enumerate(lines):
+            if not bug_found:
+                res = BUG_RE.match(line)
+                if res:
+                    bug_no = int(res.group(1))
+                    debug("Found bug skip %s on line %d", bug_no, x + 1)
+                    bug_found = True
+            else:
+                res = DEF_RE.match(line)
+                if res:
+                    method = res.group(1)
+                    debug("Found test method %s skips for bug %d",
+                          method, bug_no)
+                    results.append((method, bug_no))
+                    bug_found = False
     return results