diff --git a/HACKING.rst b/HACKING.rst index d0adf6c709..f0bcfe93eb 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -10,19 +10,12 @@ As such, we tend to follow Neutron conventions regarding coding style. Octavia Specific Commandments ----------------------------- -- [O316] Change assertTrue(isinstance(A, B)) by optimal assert like - assertIsInstance(A, B). -- [O318] Change assert(Not)Equal(A, None) or assert(Not)Equal(None, A) - by optimal assert like assertIs(Not)None(A). - [O319] Validate that debug level logs are not translated. - [O321] Validate that jsonutils module is used instead of json - [O322] Don't use author tags - [O323] Change assertEqual(True, A) or assertEqual(False, A) to the more specific assertTrue(A) or assertFalse(A) - [O324] Method's default argument shouldn't be mutable -- [O338] Change assertEqual(A in B, True), assertEqual(True, A in B), - assertEqual(A in B, False) or assertEqual(False, A in B) to the more - specific assertIn/NotIn(A, B) - [O339] LOG.warn() is not allowed. Use LOG.warning() - [O340] Don't use xrange() - [O341] Don't translate logs. diff --git a/octavia/hacking/checks.py b/octavia/hacking/checks.py index eb0abc2558..0b3bfc230c 100644 --- a/octavia/hacking/checks.py +++ b/octavia/hacking/checks.py @@ -42,24 +42,11 @@ _log_translation_hint = re.compile( hints='|'.join(_all_hints), )) -assert_trueinst_re = re.compile( - r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, " - r"(\w|\.|\'|\"|\[|\])+\)\)") -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_true_re = re.compile( r"assertEqual\(True,") assert_equal_with_false_re = re.compile( r"assertEqual\(False,") mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") -assert_equal_end_with_none_re = re.compile(r"(.)*assertEqual\(.+, None\)") -assert_equal_start_with_none_re = re.compile(r".*assertEqual\(None, .+\)") -assert_not_equal_end_with_none_re = re.compile( - r"(.)*assertNotEqual\(.+, None\)") -assert_not_equal_start_with_none_re = re.compile( - r"(.)*assertNotEqual\(None, .+\)") revert_must_have_kwargs_re = re.compile( r'[ ]*def revert\(.+,[ ](?!\*\*kwargs)\w+\):') untranslated_exception_re = re.compile(r"raise (?:\w*)\((.*)\)") @@ -73,35 +60,6 @@ def _translation_checks_not_enforced(filename): return any(pat in filename for pat in ["/tests/", "rally-jobs/plugins/"]) -@core.flake8ext -def assert_true_instance(logical_line): - """Check for assertTrue(isinstance(a, b)) sentences - - O316 - """ - if assert_trueinst_re.match(logical_line): - yield (0, "O316: assertTrue(isinstance(a, b)) sentences not allowed. " - "Use assertIsInstance instead.") - - -@core.flake8ext -def assert_equal_or_not_none(logical_line): - """Check for assertEqual(A, None) or assertEqual(None, A) sentences, - - assertNotEqual(A, None) or assertNotEqual(None, A) sentences - - O318 - """ - msg = ("O318: assertEqual/assertNotEqual(A, None) or " - "assertEqual/assertNotEqual(None, A) sentences not allowed") - res = (assert_equal_start_with_none_re.match(logical_line) or - assert_equal_end_with_none_re.match(logical_line) or - assert_not_equal_start_with_none_re.match(logical_line) or - assert_not_equal_end_with_none_re.match(logical_line)) - if res: - yield (0, msg) - - @core.flake8ext def assert_equal_true_or_false(logical_line): """Check for assertEqual(True, A) or assertEqual(False, A) sentences @@ -122,22 +80,6 @@ def no_mutable_default_args(logical_line): yield (0, msg) -@core.flake8ext -def assert_equal_in(logical_line): - """Check for assertEqual(A in B, True), assertEqual(True, A in B), - - assertEqual(A in B, False) or assertEqual(False, A in B) sentences - - O338 - """ - 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, "O338: Use assertIn/NotIn(A, B) rather than " - "assertEqual(A in B, True/False) when checking collection " - "contents.") - - @core.flake8ext def no_log_warn(logical_line): """Disallow 'LOG.warn(' diff --git a/octavia/tests/unit/hacking/test_checks.py b/octavia/tests/unit/hacking/test_checks.py index f9ccdf83cb..43fa0f3f6c 100644 --- a/octavia/tests/unit/hacking/test_checks.py +++ b/octavia/tests/unit/hacking/test_checks.py @@ -59,35 +59,6 @@ class HackingTestCase(base.BaseTestCase): def assertLineFails(self, func, *args): self.assertIsInstance(next(func(*args)), tuple) - def test_assert_true_instance(self): - self.assertEqual(1, len(list(checks.assert_true_instance( - "self.assertTrue(isinstance(e, " - "exception.BuildAbortException))")))) - - self.assertEqual(0, len(list(checks.assert_true_instance( - "self.assertTrue()")))) - - def test_assert_equal_or_not_none(self): - self.assertEqual(1, len(list(checks.assert_equal_or_not_none( - "self.assertEqual(A, None)")))) - - self.assertEqual(1, len(list(checks.assert_equal_or_not_none( - "self.assertEqual(None, A)")))) - - self.assertEqual(1, len(list(checks.assert_equal_or_not_none( - "self.assertNotEqual(A, None)")))) - - self.assertEqual(1, len(list(checks.assert_equal_or_not_none( - "self.assertNotEqual(None, A)")))) - - self.assertEqual(0, - len(list(checks.assert_equal_or_not_none( - "self.assertIsNone()")))) - - self.assertEqual(0, - len(list(checks.assert_equal_or_not_none( - "self.assertIsNotNone()")))) - def test_no_mutable_default_args(self): self.assertEqual(0, len(list(checks.no_mutable_default_args( "def foo (bar):")))) @@ -96,43 +67,6 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual(1, len(list(checks.no_mutable_default_args( "def foo (bar={}):")))) - def test_assert_equal_in(self): - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual(a in b, True)")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual('str' in 'string', True)")))) - - self.assertEqual(0, len(list(checks.assert_equal_in( - "self.assertEqual(any(a==1 for a in b), True)")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual(True, a in b)")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual(True, 'str' in 'string')")))) - - self.assertEqual(0, len(list(checks.assert_equal_in( - "self.assertEqual(True, any(a==1 for a in b))")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual(a in b, False)")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual('str' in 'string', False)")))) - - self.assertEqual(0, len(list(checks.assert_equal_in( - "self.assertEqual(any(a==1 for a in b), False)")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual(False, a in b)")))) - - self.assertEqual(1, len(list(checks.assert_equal_in( - "self.assertEqual(False, 'str' in 'string')")))) - - self.assertEqual(0, len(list(checks.assert_equal_in( - "self.assertEqual(False, any(a==1 for a in b))")))) - def test_assert_equal_true_or_false(self): self.assertEqual(1, len(list(checks.assert_equal_true_or_false( "self.assertEqual(True, A)")))) diff --git a/tox.ini b/tox.ini index e42b5c7a60..9bec482e06 100644 --- a/tox.ini +++ b/tox.ini @@ -183,11 +183,8 @@ import_exceptions = octavia.i18n [flake8:local-plugins] extension = - O316 = checks:assert_true_instance - O318 = checks:assert_equal_or_not_none O323 = checks:assert_equal_true_or_false O324 = checks:no_mutable_default_args - O338 = checks:assert_equal_in O339 = checks:no_log_warn O341 = checks:no_translate_logs O342 = checks:check_raised_localized_exceptions