Update hacking version

Update hacking to a more recent version, along with
flake8-import-order.

Remove N347 (import mock library) check as that is the
default with later hacking versions.

Update the builtins override of '_' to be the neutron.i18n
version due to the code triggering a false positive. This
is done in a couple of other projects as well.

Fix a number of new warnings it found.

Added some ignore directives for new whitespace issues
found in the test tree, can fix later.

TrivialFix

Change-Id: I5923255af86cf1fa11ab8e3b03bb9efac7dd7b58

HACKING.rst

@@ -28,7 +28,6 @@ Below you can find a list of checks specific to this repository.
 - [N344] Python 3: Do not use filter(lambda obj: test(obj), data). Replace it
   with [obj for obj in data if test(obj)].
 - [N346] Use neutron_lib.db.api.sqla_listen rather than sqlalchemy
-- [N347] Test code must not import mock library
 - [N348] Test code must not import six library
 
 .. note::

neutron/__init__.py

@@ -16,12 +16,12 @@
 import builtins
 import gettext
 
-from debtcollector import removals
+from neutron._i18n import _ as n_under
 
 
 gettext.install('neutron')
 
 
-builtins.__dict__['_'] = removals.remove(
-    message='Builtin _ translation function is deprecated in OpenStack; '
-            'use the function from _i18n module for your project.')(_)  # noqa
+# gettext will install its own translation function, override it to be
+# the one from neutron
+builtins.__dict__['_'] = n_under

neutron/db/agents_db.py

@@ -241,7 +241,7 @@ class AgentDbMixin(ext_agent.AgentPluginBase, AgentAvailabilityZoneMixin):
         configs = agent.get('configurations', {})
         load_type = None
         load = 0
-        if(agent['agent_type'] == constants.AGENT_TYPE_DHCP):
+        if (agent['agent_type'] == constants.AGENT_TYPE_DHCP):
             load_type = cfg.CONF.dhcp_load_type
         if load_type:
             load = int(configs.get(load_type, 0))

neutron/db/l3_db.py

@@ -1517,7 +1517,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
         dns_data = None
         with plugin_utils.delete_port_on_error(
                 self._core_plugin, context.elevated(),
-                external_port['id']),\
+                external_port['id']), \
                 db_api.CONTEXT_WRITER.using(context):
             # Ensure IPv4 addresses are allocated on external port
             external_ipv4_ips = self._port_ipv4_fixed_ips(external_port)

neutron/hacking/checks.py

@@ -37,8 +37,6 @@ tests_imports_dot = re.compile(r"\bimport[\s]+neutron.tests\b")
 tests_imports_from1 = re.compile(r"\bfrom[\s]+neutron.tests\b")
 tests_imports_from2 = re.compile(r"\bfrom[\s]+neutron[\s]+import[\s]+tests\b")
 
-import_mock = re.compile(r"\bimport[\s]+mock\b")
-import_from_mock = re.compile(r"\bfrom[\s]+mock[\s]+import\b")
 import_six = re.compile(r"\bimport[\s]+six\b")
 import_from_six = re.compile(r"\bfrom[\s]+six[\s]+import\b")
 
@@ -207,7 +205,7 @@ def check_no_imports_from_tests(logical_line, filename, noqa):
 
     for regex in tests_imports_dot, tests_imports_from1, tests_imports_from2:
         if re.match(regex, logical_line):
-            yield(0, msg)
+            yield (0, msg)
 
 
 @core.flake8ext
@@ -218,7 +216,7 @@ def check_python3_no_filter(logical_line):
            "filter(lambda obj: test(obj), data) on python3.")
 
     if filter_match.match(logical_line):
-        yield(0, msg)
+        yield (0, msg)
 
 
 # TODO(boden): rehome this check to neutron-lib
@@ -239,23 +237,6 @@ def check_no_sqlalchemy_event_import(logical_line, filename, noqa):
               "between unit tests")
 
 
-@core.flake8ext
-def check_no_import_mock(logical_line, filename, noqa):
-    """N347 - Test code must not import mock library
-    """
-    msg = ("N347: Test code must not import mock library")
-
-    if noqa:
-        return
-
-    if 'neutron/tests/' not in filename:
-        return
-
-    for regex in import_mock, import_from_mock:
-        if re.match(regex, logical_line):
-            yield(0, msg)
-
-
 @core.flake8ext
 def check_no_import_six(logical_line, filename, noqa):
     """N348 - Test code must not import six library
@@ -267,7 +248,7 @@ def check_no_import_six(logical_line, filename, noqa):
 
     for regex in import_six, import_from_six:
         if re.match(regex, logical_line):
-            yield(0, msg)
+            yield (0, msg)
 
 
 @core.flake8ext
@@ -281,4 +262,4 @@ def check_no_import_packaging(logical_line, filename, noqa):
 
     for regex in import_packaging, import_version_from_packaging:
         if re.match(regex, logical_line):
-            yield(0, msg)
+            yield (0, msg)

neutron/notifiers/ironic.py

@@ -78,7 +78,7 @@ class Notifier(object):
             os_exc.raise_from_response(response)
         except Exception as e:
             LOG.exception('Error encountered posting the event to '
-                          'ironic. {error}'.format(error=e))
+                          'ironic, error: %s', e)
 
     @registry.receives(resources.PORT, [events.AFTER_UPDATE])
     def process_port_update_event(self, resource, event, trigger,
@@ -102,10 +102,10 @@ class Notifier(object):
               current_port_status in [n_const.PORT_STATUS_ACTIVE,
                                       n_const.PORT_STATUS_ERROR]):
             port_event = 'bind_port'
-        LOG.debug('Queuing event for {event_type} for port {port} '
-                  'for status {status}.'.format(event_type=port_event,
-                                                port=port['id'],
-                                                status=current_port_status))
+        LOG.debug('Queuing event for %(event_type)s for port %(port)s '
+                  'for status %(status)s.', {'event_type': port_event,
+                                             'port': port['id'],
+                                             'status': current_port_status})
         if port_event:
             notify_event = {
                 'event': '.'.join([BAREMETAL_EVENT_TYPE, port_event]),
@@ -132,10 +132,10 @@ class Notifier(object):
             return
 
         port_event = 'delete_port'
-        LOG.debug('Queuing event for {event_type} for port {port} '
-                  'for status {status}.'.format(event_type=port_event,
-                                                port=port['id'],
-                                                status='DELETED'))
+        LOG.debug('Queuing event for %(event_type)s for port %(port)s '
+                  'for status %(status)s.', {'event_type': port_event,
+                                             'port': port['id'],
+                                             'status': 'DELETED'})
         notify_event = {
             'event': '.'.join([BAREMETAL_EVENT_TYPE, port_event]),
             'port_id': port['id'],

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py

@@ -647,7 +647,7 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
             options = getattr(lsp, 'options')
             for key in list(options.keys()):
                 if key not in ovn_const.OVN_ROUTER_PORT_OPTION_KEYS:
-                    del(options[key])
+                    del options[key]
             return options
         except idlutils.RowNotFound:
             return {}

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -807,7 +807,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
             for k in (ovn_const.OVN_GW_PORT_EXT_ID_KEY,
                       ovn_const.OVN_GW_NETWORK_EXT_ID_KEY):
                 if k in external_ids:
-                    del(external_ids[k])
+                    del external_ids[k]
 
             cmds.append(self._nb_idl.db_set(
                 'Logical_Router', lr.uuid, ('external_ids', external_ids)))

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -2193,7 +2193,7 @@ class OVNClient(object):
                 options[option] = value
             else:
                 try:
-                    del(options[option])
+                    del options[option]
                 except KeyError:
                     # Option not present, job done
                     pass

neutron/plugins/ml2/plugin.py

@@ -1866,12 +1866,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         if (old_vnic_type != new_vnic_type and
                 binding.vif_type != portbindings.VIF_TYPE_UNBOUND):
-            LOG.info("Attempting to change VNIC TYPE from {old_type} to "
-                     "{new_type} on port {port_id}, this operation is not "
-                     "allowed because the port is bound".format(
-                         old_type=old_vnic_type,
-                         new_type=new_vnic_type,
-                         port_id=old_port.id))
+            LOG.info("Attempting to change VNIC TYPE from %(old_type)s to "
+                     "%(new_type)s on port %(port_id)s, this operation is not "
+                     "allowed because the port is bound",
+                     {'old_type': old_vnic_type,
+                      'new_type': new_vnic_type,
+                      'port_id': old_port.id})
             raise exc.PortInUse(
                 port_id=old_port.id,
                 net_id=old_port.network_id,

neutron/services/placement_report/plugin.py

@@ -171,7 +171,7 @@ class PlacementReportPlugin(service_base.ServicePluginBase):
 
             for deferred in deferred_batch:
                 try:
-                    LOG.debug('placement client: {}'.format(deferred))
+                    LOG.debug('placement client: %s', deferred)
                     deferred.execute()
                 except Exception as e:
                     errors = True

neutron/tests/fullstack/test_l3_agent.py

@@ -289,17 +289,17 @@ class TestL3Agent(base.BaseFullStackTestCase):
         exception_requests = self._simulate_concurrent_requests_process(
             funcs, args)
 
-        if not all(type(e) == exceptions.BadRequest
+        if not all(isinstance(e, exceptions.BadRequest)
                    for e in exception_requests):
             self.fail('Unexpected exception adding interfaces to router from '
                       'different subnets overlapping')
 
-        if not len(exception_requests) >= (subnets - 1):
+        if len(exception_requests) < subnets - 1:
             self.fail('If we have tried to associate %s subnets overlapping '
                       'cidr to the router, we should have received at least '
                       '%s or %s rejected requests, but we have only received '
-                      '%s', (str(subnets), str(subnets - 1), str(subnets),
-                             str(len(exception_requests))))
+                      '%s' % (str(subnets), str(subnets - 1), str(subnets),
+                              str(len(exception_requests))))
 
 
 class TestLegacyL3Agent(TestL3Agent):

neutron/tests/functional/cmd/test_status.py

@@ -68,7 +68,7 @@ class StatusTest(base.BaseLoggingTestCase):
             self.assertEqual(
                 expected_stderr,
                 stderr.replace('\n', ''))
-            self.assertTrue(expected_result_title in stdout)
+            self.assertIn(expected_result_title, stdout)
         except exceptions.ProcessExecutionError as error:
             self.fail("neutron-status upgrade check command failed to run. "
                       "Error: %s" % error)

neutron/tests/unit/agent/l3/extensions/qos/test_fip.py

@@ -412,16 +412,16 @@ class RouterFipRateLimitMapsTestCase(base.BaseTestCase):
     def _check_policy_map_fip(self, router_id, fip_res):
         if router_id is None:
             self.assertIsNone(self.policy_map.get_router_id_by_fip(fip_res))
-            self.assertTrue(fip_res not in self.policy_map._fips_2_router)
+            self.assertNotIn(fip_res, self.policy_map._fips_2_router)
             for router_fips in self.policy_map._router_2_fips.values():
-                self.assertTrue(fip_res not in router_fips)
+                self.assertNotIn(fip_res, router_fips)
         else:
             self.assertEqual(router_id,
                              self.policy_map.get_router_id_by_fip(fip_res))
             self.assertEqual(router_id,
                              self.policy_map._fips_2_router[fip_res])
-            self.assertTrue(fip_res in
-                            self.policy_map._router_2_fips[router_id])
+            self.assertIn(fip_res,
+                          self.policy_map._router_2_fips[router_id])
 
     def test_get_router_id_by_fip(self):
         router_id_1, router_id_2 = _uuid(), _uuid()

neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py

@@ -231,7 +231,7 @@ class TestDhcpRpcCallback(base.BaseTestCase):
 
         def _make_subnet_dict(subnet):
             ret = {'id': subnet.id}
-            if type(subnet.segment_id) == str:
+            if isinstance(subnet.segment_id, str):
                 ret['segment_id'] = subnet.segment_id
             return ret
 

neutron/tests/unit/extensions/test_l3_ndp_proxy.py

@@ -277,7 +277,7 @@ class L3NDPProxyTestCase(test_address_scope.AddressScopeTestCase,
                 "The external network %s don't support IPv6 ndp proxy, the "
                 "network has no IPv6 subnets or has no IPv6 address "
                 "scope.") % ext_net['network']['id']
-            self.assertTrue(expected_msg in res['NeutronError']['message'])
+            self.assertIn(expected_msg, res['NeutronError']['message'])
             router = self._make_router(
                 self.fmt, self._tenant_id,
                 external_gateway_info={'network_id': ext_net['network']['id']})

neutron/tests/unit/extensions/test_securitygroup.py

@@ -838,7 +838,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                 self.fmt, res.get_response(self.ext_api))
 
             secgroup = group['security_group']
-            self.assertFalse('security_group_rules' in secgroup)
+            self.assertNotIn('security_group_rules', secgroup)
             self.assertEqual(remote_group_id, group['security_group']['id'])
 
     # This test case checks that admins from a different tenant can add rules

neutron/tests/unit/hacking/test_checks.py

@@ -206,25 +206,6 @@ class HackingTestCase(base.BaseTestCase):
         self.assertLinePasses(f, "filter(function, range(0,10))")
         self.assertLinePasses(f, "lambda x, y: x+y")
 
-    def test_check_no_import_mock(self):
-        pass_line = 'from unittest import mock'
-        fail_lines = ('import mock',
-                      'import mock as mock_lib',
-                      'from mock import patch')
-        self.assertEqual(
-            0, len(list(
-                checks.check_no_import_mock(
-                    pass_line, "neutron/tests/test_fake.py", None))))
-        for fail_line in fail_lines:
-            self.assertEqual(
-                0, len(list(
-                    checks.check_no_import_mock(
-                        fail_line, "neutron/common/utils.py", None))))
-            self.assertEqual(
-                1, len(list(
-                    checks.check_no_import_mock(
-                        fail_line, "neutron/tests/test_fake.py", None))))
-
     def test_check_no_import_six(self):
         pass_line = 'from other_library import six'
         fail_lines = ('import six',

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -354,7 +354,7 @@ class TestMl2NetworksV2(test_plugin.TestNetworksV2,
                 result_macs.append(port_mac)
                 for ip_addr in port.get('fixed_ips'):
                     self.assertIsNone(validators.validate_ip_address(ip_addr))
-            self.assertTrue(test_mac in result_macs)
+            self.assertIn(test_mac, result_macs)
 
     def test_bulk_network_before_and_after_events_outside_of_txn(self):
         # capture session states during each before and after event
@@ -1511,7 +1511,7 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
                         [mock.call(mock.ANY, [sg]) for sg in used_sg],
                         any_order=True)
             else:
-                self.assertTrue('ports' in ports)
+                self.assertIn('ports', ports)
 
     def test_create_ports_bulk_with_portbinding_attrs(self):
         with self.network() as net:

neutron/tests/unit/plugins/ml2/test_port_binding.py

@@ -481,8 +481,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
                                                  'host-fail')
             self.assertEqual(webob.exc.HTTPInternalServerError.code,
                              response.status_int)
-            self.assertTrue(exceptions.PortBindingError.__name__ in
-                            response.text)
+            self.assertIn(exceptions.PortBindingError.__name__, response.text)
 
     def test_create_port_binding_for_non_compute_owner(self):
         with self.port() as port:
@@ -538,8 +537,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
                                                  self.host, **kwargs)
             self.assertEqual(webob.exc.HTTPInternalServerError.code,
                              response.status_int)
-            self.assertTrue(exceptions.PortBindingError.__name__ in
-                            response.text)
+            self.assertIn(exceptions.PortBindingError.__name__, response.text)
 
     def test_activate_port_binding(self):
         port, new_binding = self._create_port_and_binding()
@@ -613,8 +611,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
             response = self._activate_port_binding(port['id'], self.host)
             self.assertEqual(webob.exc.HTTPInternalServerError.code,
                              response.status_int)
-            self.assertTrue(exceptions.PortBindingError.__name__ in
-                            response.text)
+            self.assertIn(exceptions.PortBindingError.__name__, response.text)
             self.assertEqual(ml2_plugin.MAX_BIND_TRIES, p_mock.call_count)
 
     def test_activate_port_binding_non_existing_binding(self):
@@ -687,8 +684,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
 
             self.assertEqual(webob.exc.HTTPInternalServerError.code,
                              response.status_int)
-            self.assertTrue(exceptions.PortBindingError.__name__ in
-                            response.text)
+            self.assertIn(exceptions.PortBindingError.__name__, response.text)
 
     def _create_unbound_port(self):
         with self.port() as port:

neutron/tests/unit/services/placement_report/test_plugin.py

@@ -349,6 +349,6 @@ class PlacementReporterAgentsTestCases(test_plugin.Ml2PluginV2TestCase):
     def test_mechanism_driver_by_agent_type_not_found(self):
         self.agents = plugin.PlacementReporterAgents(ml2_plugin=self.plugin)
         self.assertRaises(
-            Exception,  # noqa
+            KeyError,
             self.agents.mechanism_driver_by_agent_type,
             'agent_not_belonging_to_any_mechanism_driver')

tox.ini

@@ -26,7 +26,7 @@ deps =
   -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master}
   -r{toxinidir}/requirements.txt
   -r{toxinidir}/test-requirements.txt
-  hacking>=3.0.1,<3.1.0 # Apache-2.0
+  hacking>=6.1.0,<6.2.0 # Apache-2.0
 allowlist_externals = bash
 commands =
   bash {toxinidir}/tools/pip_install_src_modules.sh "{toxinidir}"
@@ -119,7 +119,7 @@ deps =
   {[testenv]deps}
   bashate>=2.1.1 # Apache-2.0
   bandit>=1.7.5 # Apache-2.0
-  flake8-import-order==0.18.2 # LGPLv3
+  flake8-import-order>=0.18.2,<0.19.0 # LGPLv3
   pylint==2.17.4 # GPLv2
 commands=
   # If it is easier to add a check via a shell script, consider adding it in this file
@@ -178,13 +178,15 @@ commands = sphinx-build -W -b linkcheck doc/source doc/build/linkcheck
 [flake8]
 # E126 continuation line over-indented for hanging indent
 # E128 continuation line under-indented for visual indent
+# E231 missing whitespace after ','
+# E275 missing whitespace after keyword
 # H405 multi line docstring summary not separated with an empty line
 # I202 Additional newline in a group of imports
 # N530 direct neutron imports not allowed
 # TODO(amotoki) check the following new rules should be fixed or ignored
 # E731 do not assign a lambda expression, use a def
 # W504 line break after binary operator
-ignore = E126,E128,E731,I202,H405,N530,W504
+ignore = E126,E128,E231,E275,E731,I202,H405,N530,W504
 # H106: Don't put vim configuration in source files
 # H203: Use assertIs(Not)None to check for None
 # H204: Use assert(Not)Equal to check for equality
@@ -209,7 +211,6 @@ extension =
   N343 = neutron.hacking.checks:check_no_imports_from_tests
   N344 = neutron.hacking.checks:check_python3_no_filter
   N346 = neutron.hacking.checks:check_no_sqlalchemy_event_import
-  N347 = neutron.hacking.checks:check_no_import_mock
   N348 = neutron.hacking.checks:check_no_import_six
   N349 = neutron.hacking.checks:check_no_import_packaging
   # Checks from neutron-lib