From 85a1b8d40b641ccac41abd998a830c4f02af6e6c Mon Sep 17 00:00:00 2001 From: "jeremy.zhang" Date: Wed, 14 Jun 2017 10:22:03 +0800 Subject: [PATCH] Enable some off-by-default checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of the available checks are disabled by default in flake8, like: [H106] Don’t put vim configuration in source files [H203] Use assertIs(Not)None to check for None This patch is to enable the H106 and H203 checks in Cinder project. The C312 hacking rule will be removed when turn on H203. Change-Id: I2e883c301b64d5977bbb907b63c9c144bc6f959d --- HACKING.rst | 1 - cinder/hacking/checks.py | 8 -------- cinder/tests/unit/objects/test_base.py | 2 +- cinder/tests/unit/test_hacking.py | 7 ------- cinder/tests/unit/test_quota.py | 2 +- .../drivers/netapp/dataontap/client/test_client_7mode.py | 8 ++++---- .../drivers/netapp/dataontap/client/test_client_cmode.py | 4 ++-- tox.ini | 1 + 8 files changed, 9 insertions(+), 24 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index edf37a2d34a..1add95e21be 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -24,7 +24,6 @@ Cinder Specific Commandments - [C309] Unit tests should not perform logging. - [C310] Check for improper use of logging format arguments. - [C311] Check for proper naming and usage in option registration. -- [C312] Check that assertIsNone(value) is used and not assertEqual(None, value). - [C313] Check that assertTrue(value) is used and not assertEqual(True, value). General diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py index 0ae042f9bcd..6bbb428f851 100644 --- a/cinder/hacking/checks.py +++ b/cinder/hacking/checks.py @@ -448,13 +448,6 @@ def no_test_log(logical_line, filename, noqa): yield (0, msg) -def validate_assertIsNone(logical_line): - if re.match(assert_None, logical_line): - msg = ("C312: Unit tests should use assertIsNone(value) instead" - " of using assertEqual(None, value).") - yield(0, msg) - - def validate_assertTrue(logical_line): if re.match(assert_True, logical_line): msg = ("C313: Unit tests should use assertTrue(value) instead" @@ -479,5 +472,4 @@ def factory(register): register(no_log_warn) register(dict_constructor_with_list_copy) register(no_test_log) - register(validate_assertIsNone) register(validate_assertTrue) diff --git a/cinder/tests/unit/objects/test_base.py b/cinder/tests/unit/objects/test_base.py index 54938613252..2b7bdc9cf29 100644 --- a/cinder/tests/unit/objects/test_base.py +++ b/cinder/tests/unit/objects/test_base.py @@ -179,7 +179,7 @@ class TestCinderComparableObject(test_objects.BaseObjectsTestCase): self.assertTrue(obj1 == obj2) self.assertFalse(obj1 == obj3) self.assertFalse(obj1 == obj4) - self.assertNotEqual(obj1, None) + self.assertIsNotNone(obj1) @ddt.ddt diff --git a/cinder/tests/unit/test_hacking.py b/cinder/tests/unit/test_hacking.py index b63a66fb05c..3775cafb047 100644 --- a/cinder/tests/unit/test_hacking.py +++ b/cinder/tests/unit/test_hacking.py @@ -361,13 +361,6 @@ class HackingTestCase(test.TestCase): self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( " self._render_dict(xml, data_el, data.__dict__)")))) - def test_validate_assertIsNone(self): - test_value = None - self.assertEqual(0, len(list(checks.validate_assertIsNone( - "assertIsNone(None)")))) - self.assertEqual(1, len(list(checks.validate_assertIsNone( - "assertEqual(None, %s)" % test_value)))) - def test_validate_assertTrue(self): test_value = True self.assertEqual(0, len(list(checks.validate_assertTrue( diff --git a/cinder/tests/unit/test_quota.py b/cinder/tests/unit/test_quota.py index 96337bd830a..34ea007dfb7 100644 --- a/cinder/tests/unit/test_quota.py +++ b/cinder/tests/unit/test_quota.py @@ -1827,7 +1827,7 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase): def _mock_allocated_get_all_by_project(self, allocated_quota=False): def fake_qagabp(context, project_id, session=None): self.assertEqual('test_project', project_id) - self.assertNotEqual(session, None) + self.assertIsNotNone(session) if allocated_quota: return dict(project_id=project_id, volumes=3, gigabytes = 2 * 1024) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_7mode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_7mode.py index f809d8f2838..c55669e357b 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_7mode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_7mode.py @@ -533,8 +533,8 @@ class NetApp7modeClientTestCase(test.TestCase): self.assertEqual( fake.CG_SNAPSHOT_ID, actual_request.get_child_by_name('snapshot-name').get_content()) - self.assertEqual(actual_request.get_child_by_name( - 'destination-exists'), None) + self.assertIsNone(actual_request.get_child_by_name( + 'destination-exists')) self.assertTrue(enable_tunneling) def test_clone_file_when_clone_fails(self): @@ -589,8 +589,8 @@ class NetApp7modeClientTestCase(test.TestCase): self.assertEqual(expected_src_path, actual_src_path) self.assertEqual(expected_dest_path, actual_dest_path) - self.assertEqual(actual_request.get_child_by_name( - 'destination-exists'), None) + self.assertIsNone(actual_request.get_child_by_name( + 'destination-exists')) self.assertTrue(enable_tunneling) __, _args, _kwargs = self.connection.invoke_successfully.mock_calls[1] diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py index eafb8ae87b5..64ac143edf5 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py @@ -1146,8 +1146,8 @@ class NetAppCmodeClientTestCase(test.TestCase): req_snapshot_child = actual_request.get_child_by_name('snapshot-name') self.assertEqual(fake.CG_SNAPSHOT_ID, req_snapshot_child.get_content()) - self.assertEqual(actual_request.get_child_by_name( - 'destination-exists'), None) + self.assertIsNone(actual_request.get_child_by_name( + 'destination-exists')) def test_clone_file_when_destination_exists(self): expected_flex_vol = "fake_flex_vol" diff --git a/tox.ini b/tox.ini index 71647777690..2fc874cb967 100644 --- a/tox.ini +++ b/tox.ini @@ -129,6 +129,7 @@ usedevelop = False # E251 unexpected spaces around keyword / parameter equals # reason: no improvement in readability ignore = E251 +enable-extensions = H106,H203 exclude = .git,.venv,.tox,dist,tools,doc/ext,*egg,build max-complexity=30