Bump hacking

hacking was indirectly capped by pycodestyle. This bumps hacking to
apply the rules recently added.

Also remove the note about pip's behavior, which is no longer valid
for recent versions.

notes:
 - T117 test is now disabled. There are a lot of lines violating
   this rule and we have to decide if we really want to enforce it.

 - Once this is merged, we have to update bump hacking in some plugins
   which import hacking extensions from tempest.

Change-Id: I5ee5e152418079f9f2720eb97c3a5361edba2695

requirements.txt

@@ -1,6 +1,3 @@
-# The order of packages is significant, because pip processes them in the order
-# of appearance. Changing the order has an impact on the overall integration
-# process, which may cause wedges in the gate later.
 pbr!=2.1.0,>=2.0.0 # Apache-2.0
 cliff!=2.9.0,>=2.8.0 # Apache-2.0
 jsonschema>=3.2.0 # MIT

tempest/api/identity/admin/v3/test_groups.py

@@ -128,7 +128,7 @@ class GroupsV3TestJSON(base.BaseIdentityV3AdminTest):
         for g in user_groups:
             if 'membership_expires_at' in g:
                 self.assertIsNone(g['membership_expires_at'])
-                del(g['membership_expires_at'])
+                del g['membership_expires_at']
         self.assertEqual(sorted(groups, key=lambda k: k['name']),
                          sorted(user_groups, key=lambda k: k['name']))
         self.assertEqual(2, len(user_groups))

tempest/common/utils/linux/remote_client.py

@@ -59,14 +59,14 @@ class RemoteClient(remote_client.RemoteClient):
         output = self.exec_command(command)
         selected = []
         pos = None
-        for l in output.splitlines():
+        for line in output.splitlines():
-            if pos is None and l.find("TYPE") > 0:
+            if pos is None and line.find("TYPE") > 0:
-                pos = l.find("TYPE")
+                pos = line.find("TYPE")
                 # Show header line too
-                selected.append(l)
+                selected.append(line)
             # lsblk lists disk type in a column right-aligned with TYPE
-            elif pos is not None and pos > 0 and l[pos:pos + 4] == "disk":
+            elif pos is not None and pos > 0 and line[pos:pos + 4] == "disk":
-                selected.append(l)
+                selected.append(line)
 
         if selected:
             return "\n".join(selected)
@@ -121,9 +121,9 @@ class RemoteClient(remote_client.RemoteClient):
     def _get_dns_servers(self):
         cmd = 'cat /etc/resolv.conf'
         resolve_file = self.exec_command(cmd).strip().split('\n')
-        entries = (l.split() for l in resolve_file)
+        entries = (line.split() for line in resolve_file)
-        dns_servers = [l[1] for l in entries
+        dns_servers = [line[1] for line in entries
-                       if len(l) and l[0] == 'nameserver']
+                       if len(line) and line[0] == 'nameserver']
         return dns_servers
 
     def get_dns_servers(self, timeout=5):

tempest/common/utils/net_downtime.py

@@ -50,10 +50,10 @@ done
 
 
 class NetDowntimeMeter(fixtures.Fixture):
-    def __init__(self, dest_ip, interval='0.2'):
+    def __init__(self, dest_ip, interval=0.2):
         self.dest_ip = dest_ip
         # Note: for intervals lower than 0.2 ping requires root privileges
-        self.interval = interval
+        self.interval = float(interval)
         self.ping_process = None
 
     def _setUp(self):
@@ -61,18 +61,18 @@ class NetDowntimeMeter(fixtures.Fixture):
 
     def start_background_pinger(self):
         cmd = ['ping', '-q', '-s1']
-        cmd.append('-i{}'.format(self.interval))
+        cmd.append('-i%g' % self.interval)
         cmd.append(self.dest_ip)
-        LOG.debug("Starting background pinger to '{}' with interval {}".format(
+        LOG.debug("Starting background pinger to '%s' with interval %g",
-            self.dest_ip, self.interval))
+                  self.dest_ip, self.interval)
         self.ping_process = subprocess.Popen(
             cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         self.addCleanup(self.cleanup)
 
     def cleanup(self):
         if self.ping_process and self.ping_process.poll() is None:
-            LOG.debug('Terminating background pinger with pid {}'.format(
+            LOG.debug('Terminating background pinger with pid %d',
-                self.ping_process.pid))
+                      self.ping_process.pid)
             self.ping_process.terminate()
         self.ping_process = None
 
@@ -83,7 +83,7 @@ class NetDowntimeMeter(fixtures.Fixture):
         output = self.ping_process.stderr.readline().strip().decode('utf-8')
         if output and len(output.split()[0].split('/')) == 2:
             succ, total = output.split()[0].split('/')
-            return (int(total) - int(succ)) * float(self.interval)
+            return (int(total) - int(succ)) * self.interval
         else:
             LOG.warning('Unexpected output obtained from the pinger: %s',
                         output)
@@ -115,8 +115,9 @@ class MetadataDowntimeMeter(fixtures.Fixture):
         chmod_cmd = 'chmod +x {}'.format(self.script_path)
         self.ssh_client.exec_command(';'.join((echo_cmd, chmod_cmd)))
         LOG.debug('script created: %s', self.script_path)
-        LOG.debug(self.ssh_client.exec_command(
+        output = self.ssh_client.exec_command(
-            'cat {}'.format(self.script_path)))
+            'cat {}'.format(self.script_path))
+        LOG.debug('script content: %s', output)
 
     def run_metadata_script(self):
         self.ssh_client.exec_command('{} > {} &'.format(self.script_path,

tempest/common/waiters.py

@@ -327,8 +327,7 @@ def wait_for_image_deleted_from_store(client, image, available_stores,
     # Check if image have last store location
     if len(available_stores) == 1:
         exc_cls = lib_exc.OtherRestClientException
-        message = ('Delete from last store location not allowed'
+        message = 'Delete from last store location not allowed'
-                   % (image, image_store_deleted))
         raise exc_cls(message)
     start = int(time.time())
     while int(time.time()) - start < client.build_timeout:
@@ -548,7 +547,7 @@ def wait_for_interface_status(client, server_id, port_id, status):
     interface_status = body['port_state']
     start = int(time.time())
 
-    while(interface_status != status):
+    while interface_status != status:
         time.sleep(client.build_interval)
         body = (client.show_interface(server_id, port_id)
                 ['interfaceAttachment'])

tempest/hacking/checks.py

@@ -16,7 +16,6 @@ import os
 import re
 
 from hacking import core
-import pycodestyle
 
 
 PYTHON_CLIENTS = ['cinder', 'glance', 'keystone', 'nova', 'swift', 'neutron',
@@ -40,22 +39,22 @@ _HAVE_NEGATIVE_DECORATOR = False
 
 
 @core.flake8ext
-def import_no_clients_in_api_and_scenario_tests(physical_line, filename):
+def import_no_clients_in_api_and_scenario_tests(logical_line, filename):
     """Check for client imports from tempest/api & tempest/scenario tests
 
     T102: Cannot import OpenStack python clients
     """
 
     if "tempest/api" in filename or "tempest/scenario" in filename:
-        res = PYTHON_CLIENT_RE.match(physical_line)
+        res = PYTHON_CLIENT_RE.match(logical_line)
         if res:
-            return (physical_line.find(res.group(1)),
+            return (logical_line.find(res.group(1)),
                     ("T102: python clients import not allowed"
                      " in tempest/api/* or tempest/scenario/* tests"))
 
 
 @core.flake8ext
-def scenario_tests_need_service_tags(physical_line, filename,
+def scenario_tests_need_service_tags(logical_line, filename,
                                      previous_logical):
     """Check that scenario tests have service tags
 
@@ -63,28 +62,28 @@ def scenario_tests_need_service_tags(physical_line, filename,
     """
 
     if 'tempest/scenario/' in filename and '/test_' in filename:
-        if TEST_DEFINITION.match(physical_line):
+        if TEST_DEFINITION.match(logical_line):
             if not SCENARIO_DECORATOR.match(previous_logical):
-                return (physical_line.find('def'),
+                return (logical_line.find('def'),
                         "T104: Scenario tests require a service decorator")
 
 
 @core.flake8ext
-def no_setup_teardown_class_for_tests(physical_line, filename):
+def no_setup_teardown_class_for_tests(logical_line, filename, noqa):
 
-    if pycodestyle.noqa(physical_line):
+    if noqa:
         return
 
     if 'tempest/test.py' in filename or 'tempest/lib/' in filename:
         return
 
-    if SETUP_TEARDOWN_CLASS_DEFINITION.match(physical_line):
+    if SETUP_TEARDOWN_CLASS_DEFINITION.match(logical_line):
-        return (physical_line.find('def'),
+        return (logical_line.find('def'),
                 "T105: (setUp|tearDown)Class can not be used in tests")
 
 
 @core.flake8ext
-def service_tags_not_in_module_path(physical_line, filename):
+def service_tags_not_in_module_path(logical_line, filename):
     """Check that a service tag isn't in the module path
 
     A service tag should only be added if the service name isn't already in
@@ -96,14 +95,14 @@ def service_tags_not_in_module_path(physical_line, filename):
     # created for services like heat which would cause false negatives for
     # those tests, so just exclude the scenario tests.
     if 'tempest/scenario' not in filename:
-        matches = SCENARIO_DECORATOR.match(physical_line)
+        matches = SCENARIO_DECORATOR.match(logical_line)
         if matches:
             services = matches.group(1).split(',')
             for service in services:
                 service_name = service.strip().strip("'")
                 modulepath = os.path.split(filename)[0]
                 if service_name in modulepath:
-                    return (physical_line.find(service_name),
+                    return (logical_line.find(service_name),
                             "T107: service tag should not be in path")
 
 
@@ -140,28 +139,27 @@ def no_testtools_skip_decorator(logical_line):
                "decorators.skip_because from tempest.lib")
 
 
-def _common_service_clients_check(logical_line, physical_line, filename):
+def _common_service_clients_check(logical_line, filename, noqa):
+    if noqa:
+        return False
+
     if not re.match('tempest/(lib/)?services/.*', filename):
         return False
 
-    if not METHOD.match(physical_line):
+    if not METHOD.match(logical_line):
-        return False
-
-    if pycodestyle.noqa(physical_line):
         return False
 
     return True
 
 
 @core.flake8ext
-def get_resources_on_service_clients(physical_line, logical_line, filename,
+def get_resources_on_service_clients(logical_line, filename,
-                                     line_number, lines):
+                                     line_number, lines, noqa):
     """Check that service client names of GET should be consistent
 
     T110
     """
-    if not _common_service_clients_check(logical_line, physical_line,
+    if not _common_service_clients_check(logical_line, filename, noqa):
-                                         filename):
         return
 
     for line in lines[line_number:]:
@@ -182,14 +180,13 @@ def get_resources_on_service_clients(physical_line, logical_line, filename,
 
 
 @core.flake8ext
-def delete_resources_on_service_clients(physical_line, logical_line, filename,
+def delete_resources_on_service_clients(logical_line, filename,
-                                        line_number, lines):
+                                        line_number, lines, noqa):
     """Check that service client names of DELETE should be consistent
 
     T111
     """
-    if not _common_service_clients_check(logical_line, physical_line,
+    if not _common_service_clients_check(logical_line, filename, noqa):
-                                         filename):
         return
 
     for line in lines[line_number:]:
@@ -262,7 +259,7 @@ def dont_use_config_in_tempest_lib(logical_line, filename):
             'oslo_config' in logical_line):
         msg = ('T114: tempest.lib can not have any dependency on tempest '
                'config.')
-        yield(0, msg)
+        yield (0, msg)
 
 
 @core.flake8ext
@@ -281,7 +278,7 @@ def dont_put_admin_tests_on_nonadmin_path(logical_line,
 
     if not re.match(r'.\/tempest\/api\/.*\/admin\/.*', filename):
         msg = 'T115: All admin tests should exist under admin path.'
-        yield(0, msg)
+        yield (0, msg)
 
 
 @core.flake8ext
@@ -293,11 +290,11 @@ def unsupported_exception_attribute_PY3(logical_line):
     result = EX_ATTRIBUTE.search(logical_line)
     msg = ("[T116] Unsupported 'message' Exception attribute in PY3")
     if result:
-        yield(0, msg)
+        yield (0, msg)
 
 
 @core.flake8ext
-def negative_test_attribute_always_applied_to_negative_tests(physical_line,
+def negative_test_attribute_always_applied_to_negative_tests(logical_line,
                                                              filename):
     """Check ``@decorators.attr(type=['negative'])`` applied to negative tests.
 
@@ -307,13 +304,13 @@ def negative_test_attribute_always_applied_to_negative_tests(physical_line,
 
     if re.match(r'.\/tempest\/api\/.*_negative.*', filename):
 
-        if NEGATIVE_TEST_DECORATOR.match(physical_line):
+        if NEGATIVE_TEST_DECORATOR.match(logical_line):
             _HAVE_NEGATIVE_DECORATOR = True
             return
 
-        if TEST_DEFINITION.match(physical_line):
+        if TEST_DEFINITION.match(logical_line):
             if not _HAVE_NEGATIVE_DECORATOR:
-                return (
+                yield (
                     0, "T117: Must apply `@decorators.attr(type=['negative'])`"
                        " to all negative API tests"
                 )

tempest/scenario/manager.py

@@ -851,7 +851,7 @@ class ScenarioTest(tempest.test.BaseTestCase):
                     kernel_img_path = os.path.join(extract_dir, fname)
                 elif re.search(r'(.*-initrd.*|ari-.*/image$)', fname):
                     ramdisk_img_path = os.path.join(extract_dir, fname)
-                elif re.search(f'(.*\\.img$|ami-.*/image$)', fname):
+                elif re.search(r'(.*\\.img$|ami-.*/image$)', fname):
                     img_path = os.path.join(extract_dir, fname)
             # Create the kernel image.
             kparams = {
@@ -1561,8 +1561,8 @@ class NetworkScenarioTest(ScenarioTest):
             floating_ip = (self.floating_ips_client.
                            show_floatingip(floatingip_id)['floatingip'])
             if status == floating_ip['status']:
-                LOG.info("FloatingIP: {fp} is at status: {st}"
+                LOG.info("FloatingIP: %s is at status: %s",
-                         .format(fp=floating_ip, st=status))
+                         floating_ip, status)
             return status == floating_ip['status']
 
         if not test_utils.call_until_true(refresh,

tempest/tests/test_hacking.py

@@ -51,25 +51,34 @@ class HackingTestCase(base.TestCase):
 
     def test_no_setup_teardown_class_for_tests(self):
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/tests/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/tests/fake_test.py', False))
         self.assertIsNone(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls): # noqa", './tempest/tests/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/tests/fake_test.py',
+            True))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/api/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/api/fake_test.py',
+            False))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/scenario/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/scenario/fake_test.py',
+            False))
         self.assertFalse(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/test.py'))
+            "  def setUpClass(cls):", './tempest/test.py',
+            False))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/tests/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/tests/fake_test.py',
+            False))
         self.assertIsNone(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls): # noqa", './tempest/tests/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/tests/fake_test.py',
+            True))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/api/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/api/fake_test.py',
+            False))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/scenario/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/scenario/fake_test.py',
+            False))
         self.assertFalse(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/test.py'))
+            "  def tearDownClass(cls):", './tempest/test.py',
+            False))
 
     def test_import_no_clients_in_api_and_scenario_tests(self):
         for client in checks.PYTHON_CLIENTS:
@@ -198,22 +207,26 @@ class HackingTestCase(base.TestCase):
             # arbitrarily many decorators. These insert decorators above the
             # @decorators.attr(type=['negative']) decorator.
             for decorator in other_decorators:
-                self.assertIsNone(check(" %s" % decorator, filename))
+                self.assertFalse(
+                    list(check(" %s" % decorator, filename)))
         if with_negative_decorator:
-            self.assertIsNone(
+            self.assertFalse(
-                check("@decorators.attr(type=['negative'])", filename))
+                list(check("@decorators.attr(type=['negative'])", filename)))
         if with_other_decorators:
             # Include multiple decorators to verify that this check works with
             # arbitrarily many decorators. These insert decorators between
             # the test and the @decorators.attr(type=['negative']) decorator.
             for decorator in other_decorators:
-                self.assertIsNone(check(" %s" % decorator, filename))
+                self.assertFalse(
-        final_result = check(" def test_some_negative_case", filename)
+                    list(check(" %s" % decorator, filename)))
+        final_result = list(check(" def test_some_negative_case", filename))
         if expected_success:
-            self.assertIsNone(final_result)
+            self.assertFalse(final_result)
         else:
-            self.assertIsInstance(final_result, tuple)
+            self.assertEqual(1, len(final_result))
-            self.assertFalse(final_result[0])
+            self.assertIsInstance(final_result[0], tuple)
+            self.assertEqual(0, final_result[0][0])
+            self.assertTrue(final_result[0][1])
 
     def test_no_negatve_test_attribute_applied_to_negative_test(self):
         # Check negative filename, negative decorator passes

tempest/tests/test_test.py

@@ -303,7 +303,7 @@ class TestTempestBaseTestClass(base.TestCase):
         # [0]: test, err, details [1] -> exc_info
         # Type, Exception, traceback [1] -> MultipleException
         found_exc = log[0][1][1]
-        self.assertTrue(isinstance(found_exc, testtools.MultipleExceptions))
+        self.assertIsInstance(found_exc, testtools.MultipleExceptions)
         self.assertEqual(2, len(found_exc.args))
         # Each arg is exc_info - match messages and order
         self.assertIn('mock3 resource', str(found_exc.args[0][1]))
@@ -332,7 +332,7 @@ class TestTempestBaseTestClass(base.TestCase):
         # [0]: test, err, details [1] -> exc_info
         # Type, Exception, traceback [1] -> RuntimeError
         found_exc = log[0][1][1]
-        self.assertTrue(isinstance(found_exc, RuntimeError))
+        self.assertIsInstance(found_exc, RuntimeError)
         self.assertIn(BadResourceCleanup.__name__, str(found_exc))
 
     def test_super_skip_checks_not_invoked(self):

test-requirements.txt

@@ -1,8 +1,4 @@
-# The order of packages is significant, because pip processes them in the order
+hacking>=6.1.0,<6.2.0
-# of appearance. Changing the order has an impact on the overall integration
-# process, which may cause wedges in the gate later.
-hacking>=3.0.1,<3.1.0;python_version>='3.5' # Apache-2.0
 coverage!=4.4,>=4.0 # Apache-2.0
 oslotest>=3.2.0 # Apache-2.0
-pycodestyle>=2.0.0,<2.6.0 # MIT
+flake8-import-order>=0.18.0,<0.19.0 # LGPLv3
-flake8-import-order==0.11 # LGPLv3

tox.ini

@@ -411,7 +411,8 @@ import_exceptions = tempest.services
 # E129 skipped because it is too limiting when combined with other rules
 # W504 skipped because it is overeager and unnecessary
 # H405 skipped because it arbitrarily forces doctring "title" lines
-ignore = E125,E123,E129,W504,H405
+# I201 and I202 skipped because the rule does not allow new line between 3rd party modules and own modules
+ignore = E125,E123,E129,W504,H405,I201,I202,T117
 show-source = True
 exclude = .git,.venv,.tox,dist,doc,*egg,build
 enable-extensions = H106,H203,H904