From 7f619fec8e8a2fbcaff834edddc1e4e64f7da7e0 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 19 Feb 2026 01:09:34 +0900 Subject: [PATCH] Bump hacking ... because hacking 3.0.x is too old. Also remove a few redundant checks. - M302 is checking an invalid statement - M316 is replaced by H211 - M338 is replaced by H215 - M366 is replaced by H216 - M352 is replaced by H905 - M339 is no longer required in Python 3 Change-Id: I6c289a39ca4f979e1acc869459984898e21c9076 Signed-off-by: Takashi Kajinami --- cyborg/common/authorize_wsgi.py | 2 - cyborg/hacking/checks.py | 98 ------------------- cyborg/objects/extarq/fpga_ext_arq.py | 2 +- .../drivers/nic/intel/prepare_test_data.py | 14 +-- .../drivers/nic/intel/test_driver.py | 4 +- .../drivers/qat/intel/prepare_test_data.py | 14 +-- cyborg/tests/unit/test_exception.py | 2 +- cyborg/tests/unit/test_hacking.py | 93 ------------------ test-requirements.txt | 2 +- tox.ini | 6 -- 10 files changed, 19 insertions(+), 218 deletions(-) diff --git a/cyborg/common/authorize_wsgi.py b/cyborg/common/authorize_wsgi.py index 590157c1..98a92417 100644 --- a/cyborg/common/authorize_wsgi.py +++ b/cyborg/common/authorize_wsgi.py @@ -68,8 +68,6 @@ def init_enforcer(policy_file=None, rules=None, def get_enforcer(): """Provides access to the single accelerator of policy enforcer.""" - global _ENFORCER - if not _ENFORCER: init_enforcer() diff --git a/cyborg/hacking/checks.py b/cyborg/hacking/checks.py index 4fa79f3e..e11a23cc 100644 --- a/cyborg/hacking/checks.py +++ b/cyborg/hacking/checks.py @@ -34,18 +34,7 @@ Guidelines for writing new hacking checks UNDERSCORE_IMPORT_FILES = [] mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") -assert_equal_in_end_with_true_or_false_re = re.compile( - r"assertEqual\((\w|[][.'\"])+ in (\w|[][.'\", ])+, (True|False)\)") -assert_equal_in_start_with_true_or_false_re = re.compile( - r"assertEqual\((True|False), (\w|[][.'\"])+ in (\w|[][.'\", ])+\)") -assert_equal_with_is_not_none_re = re.compile( - r"assertEqual\(.*?\s+is+\s+not+\s+None\)$") -assert_true_isinstance_re = re.compile( - r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, " - r"(\w|\.|\'|\"|\[|\])+\)\)") dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") -assert_xrange_re = re.compile( - r"\s*xrange\s*\(") log_translation = re.compile( r"(.)*LOG\.(audit|error|critical)\(\s*('|\")") log_translation_info = re.compile( @@ -69,50 +58,6 @@ def no_mutable_default_args(logical_line): yield (0, msg) -@core.flake8ext -def assert_equal_not_none(logical_line): - """Check for assertEqual(A is not None) sentences M302""" - msg = "M302: assertEqual(A is not None) sentences not allowed." - res = assert_equal_with_is_not_none_re.search(logical_line) - if res: - yield (0, msg) - - -@core.flake8ext -def assert_true_isinstance(logical_line): - """Check for assertTrue(isinstance(a, b)) sentences - - M316 - """ - if assert_true_isinstance_re.match(logical_line): - yield (0, "M316: assertTrue(isinstance(a, b)) sentences not allowed") - - -@core.flake8ext -def assert_equal_in(logical_line): - """Check for assertEqual(True|False, A in B), - assertEqual(A in B, True|False) - - M338 - """ - res = (assert_equal_in_start_with_true_or_false_re.search(logical_line) or - assert_equal_in_end_with_true_or_false_re.search(logical_line)) - if res: - yield (0, "M338: Use assertIn/NotIn(A, B) rather than " - "assertEqual(A in B, True/False) when checking collection " - "contents.") - - -@core.flake8ext -def no_xrange(logical_line): - """Disallow 'xrange()' - - M339 - """ - if assert_xrange_re.match(logical_line): - yield (0, "M339: Do not use xrange().") - - @core.flake8ext def use_timeutils_utcnow(logical_line, filename): # tools are OK to use the standard datetime module @@ -136,21 +81,6 @@ def dict_constructor_with_list_copy(logical_line): yield (0, msg) -@core.flake8ext -def no_log_warn(logical_line): - """Disallow 'LOG.warn(' - - Deprecated LOG.warn(), instead use LOG.warning - https://bugs.launchpad.net/magnum/+bug/1508442 - - M352 - """ - - msg = ("M352: LOG.warn is deprecated, please use LOG.warning!") - if "LOG.warn(" in logical_line: - yield (0, msg) - - @core.flake8ext def check_explicit_underscore_import(logical_line, filename): """Check for explicit import of the _ function @@ -171,31 +101,3 @@ def check_explicit_underscore_import(logical_line, filename): elif (translated_log.match(logical_line) or string_translation.match(logical_line)): yield (0, "M340: Found use of _() without explicit import of _ !") - - -@core.flake8ext -def import_stock_mock(logical_line): - """Use python's mock, not the mock library. - - Since we `dropped support for python 2`__, we no longer need to use the - mock library, which existed to backport py3 functionality into py2. - Which must be done by saying:: - - from unittest import mock - - ...because if you say:: - - import mock - - ...you definitely will not be getting the standard library mock. That will - always import the third party mock library. This check can be removed in - the future (and we can start saying ``import mock`` again) if we manage to - purge these transitive dependencies. - - .. __: https://review.opendev.org/#/c/688593/ - - N366 - """ - if logical_line == 'import mock': - yield (0, "N366: You must explicitly import python's mock: " - "``from unittest import mock``") diff --git a/cyborg/objects/extarq/fpga_ext_arq.py b/cyborg/objects/extarq/fpga_ext_arq.py index 665d0ec5..c9fa05b5 100644 --- a/cyborg/objects/extarq/fpga_ext_arq.py +++ b/cyborg/objects/extarq/fpga_ext_arq.py @@ -70,7 +70,7 @@ class FPGAExtARQ(ExtARQ): resp = conn.image.get('/images', params=properties) if resp: image_list = resp.json()['images'] - if type(image_list) != list: + if not isinstance(image_list, list): raise exception.InvalidType( obj='image', type=type(image_list), expected='list') diff --git a/cyborg/tests/unit/accelerator/drivers/nic/intel/prepare_test_data.py b/cyborg/tests/unit/accelerator/drivers/nic/intel/prepare_test_data.py index cd42bfea..49a133f6 100644 --- a/cyborg/tests/unit/accelerator/drivers/nic/intel/prepare_test_data.py +++ b/cyborg/tests/unit/accelerator/drivers/nic/intel/prepare_test_data.py @@ -174,16 +174,16 @@ NIC_DEVICE_VF_SOFT_LINK = { def gen_nic_content(path, dev): content = copy.copy(NIC_DEVICE_COMMON_CONTENT) content.update(NIC_DEVICES_SPECIAL_COMMON_CONTENT[dev]) - for k, v in content.items(): - p = os.path.join(path, k) - if not v: + for key, value in content.items(): + p = os.path.join(path, key) + if not value: os.mknod(p) - elif type(v) is str: + elif type(value) is str: with open(p, 'a') as f: - f.write(v + "\n") - elif type(v) is list: + f.write(value + "\n") + elif type(value) is list: with open(p, 'a') as f: - f.writelines([l + "\n" for l in v]) + f.writelines([line + "\n" for line in value]) def gen_nic_sub_dir(path): diff --git a/cyborg/tests/unit/accelerator/drivers/nic/intel/test_driver.py b/cyborg/tests/unit/accelerator/drivers/nic/intel/test_driver.py index c44771e9..98809312 100644 --- a/cyborg/tests/unit/accelerator/drivers/nic/intel/test_driver.py +++ b/cyborg/tests/unit/accelerator/drivers/nic/intel/test_driver.py @@ -86,7 +86,7 @@ class TestIntelNICDriver(base.TestCase): {'num_accelerators': 1, 'name': '0000:05:01.0', 'attach_handle_list': attach_handle_list[0], - 'attribute_list':attribute_list[0] + 'attribute_list': attribute_list[0] }, ], 'controlpath_id': @@ -104,7 +104,7 @@ class TestIntelNICDriver(base.TestCase): {'num_accelerators': 1, 'name': '0000:06:00.0', 'attach_handle_list': attach_handle_list[1], - 'attribute_list':attribute_list[1] + 'attribute_list': attribute_list[1] }, ], 'controlpath_id': diff --git a/cyborg/tests/unit/accelerator/drivers/qat/intel/prepare_test_data.py b/cyborg/tests/unit/accelerator/drivers/qat/intel/prepare_test_data.py index 268f6174..00bf2fad 100755 --- a/cyborg/tests/unit/accelerator/drivers/qat/intel/prepare_test_data.py +++ b/cyborg/tests/unit/accelerator/drivers/qat/intel/prepare_test_data.py @@ -175,16 +175,16 @@ QAT_DEVICE_VF_SOFT_LINK = { def gen_qat_content(path, dev): content = copy.copy(QAT_DEVICE_COMMON_CONTENT) content.update(QAT_DEVICES_SPECIAL_COMMON_CONTENT[dev]) - for k, v in content.items(): - p = os.path.join(path, k) - if not v: + for key, value in content.items(): + p = os.path.join(path, key) + if not value: os.mknod(p) - elif type(v) is str: + elif type(value) is str: with open(p, 'a') as f: - f.write(v + "\n") - elif type(v) is list: + f.write(value + "\n") + elif type(value) is list: with open(p, 'a') as f: - f.writelines([l + "\n" for l in v]) + f.writelines([line + "\n" for line in value]) def gen_qat_sub_dir(path): diff --git a/cyborg/tests/unit/test_exception.py b/cyborg/tests/unit/test_exception.py index d29f3879..344ee87d 100644 --- a/cyborg/tests/unit/test_exception.py +++ b/cyborg/tests/unit/test_exception.py @@ -26,7 +26,7 @@ class TestException(base.TestCase): item = getattr(exception, name) if item == base_class: continue - elif type(item) != type: + elif type(item) is not type: continue elif not issubclass(item, base_class): continue diff --git a/cyborg/tests/unit/test_hacking.py b/cyborg/tests/unit/test_hacking.py index 5e5718f1..db8fbb93 100644 --- a/cyborg/tests/unit/test_hacking.py +++ b/cyborg/tests/unit/test_hacking.py @@ -77,46 +77,6 @@ class HackingTestCase(base.TestCase): def _assert_has_no_errors(self, code, checker, filename=None): self._assert_has_errors(code, checker, filename=filename) - def test_assert_equal_in(self): - errors = [(1, 0, "M338")] - check = checks.assert_equal_in - - code = "self.assertEqual(a in b, True)" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual('str' in 'string', True)" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual(any(a==1 for a in b), True)" - self._assert_has_no_errors(code, check) - - code = "self.assertEqual(True, a in b)" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual(True, 'str' in 'string')" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual(True, any(a==1 for a in b))" - self._assert_has_no_errors(code, check) - - code = "self.assertEqual(a in b, False)" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual('str' in 'string', False)" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual(any(a==1 for a in b), False)" - self._assert_has_no_errors(code, check) - - code = "self.assertEqual(False, a in b)" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual(False, 'str' in 'string')" - self._assert_has_errors(code, check, errors) - - code = "self.assertEqual(False, any(a==1 for a in b))" - self._assert_has_no_errors(code, check) - def test_no_mutable_default_args(self): errors = [(1, 0, "M322")] check = checks.no_mutable_default_args @@ -130,49 +90,6 @@ class HackingTestCase(base.TestCase): code = "defined, undefined = [], {}" self._assert_has_no_errors(code, check) - def test_assert_is_not_none(self): - errors = [(1, 0, "M302")] - check = checks.assert_equal_not_none - - code = "self.assertEqual(A is not None)" - self._assert_has_errors(code, check, errors) - - code = "self.assertIsNotNone()" - self._assert_has_no_errors(code, check) - - def test_assert_true_isinstance(self): - errors = [(1, 0, "M316")] - check = checks.assert_true_isinstance - - code = "self.assertTrue(isinstance(e, exception.BuilAbortException))" - self._assert_has_errors(code, check, errors) - - code = "self.assertTrue()" - self._assert_has_no_errors(code, check) - - def test_no_xrange(self): - errors = [(1, 0, "M339")] - check = checks.no_xrange - - code = "xrange(45)" - self._assert_has_errors(code, check, errors) - - code = "range(45)" - self._assert_has_no_errors(code, check) - - def test_no_log_warn(self): - errors = [(1, 0, "M352")] - check = checks.no_log_warn - code = """ - LOG.warn("LOG.warn is deprecated") - """ - self._assert_has_errors(code, check, errors) - - code = """ - LOG.warning("LOG.warn is deprecated") - """ - self._assert_has_no_errors(code, check) - def test_use_timeunitls_utcow(self): errors = [(1, 0, "M310")] check = checks.use_timeutils_utcnow @@ -245,13 +162,3 @@ class HackingTestCase(base.TestCase): self.assertEqual(len(list(checks.check_explicit_underscore_import( "msg = _('My message')", "magnum/tests/other_files3.py"))), 0) - - def test_import_stock_mock(self): - self._assert_has_errors( - "import mock", - checks.import_stock_mock, expected_errors=[(1, 0, 'N366')]) - code = """ - from unittest import mock - import unittest.mock - """ - self._assert_has_no_errors(code, checks.import_stock_mock) diff --git a/test-requirements.txt b/test-requirements.txt index dff9ef8b..11f7cc80 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,4 +1,4 @@ -hacking>=3.0.1,<3.1.0 # Apache-2.0 +hacking>=8.0.0,<8.1.0 # Apache-2.0 bandit>=1.6.0 # Apache-2.0 coverage>=3.6,!=4.4 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 2a56f650..7ad64412 100644 --- a/tox.ini +++ b/tox.ini @@ -145,14 +145,8 @@ commands = bandit -r cyborg -x cyborg/tests/* -n 5 -ll [flake8:local-plugins] extension = - M302 = checks:assert_equal_not_none M310 = checks:use_timeutils_utcnow - M316 = checks:assert_true_isinstance M322 = checks:no_mutable_default_args M336 = checks:dict_constructor_with_list_copy - M338 = checks:assert_equal_in - M339 = checks:no_xrange M340 = checks:check_explicit_underscore_import - M352 = checks:no_log_warn - N366 = checks:import_stock_mock paths = ./cyborg/hacking