summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJenkins <jenkins@review.openstack.org>2017-08-08 00:15:02 +0000
committerGerrit Code Review <review@openstack.org>2017-08-08 00:15:02 +0000
commit4ce94ec6ed9afa8e65069aa64d4ec5089af76b90 (patch)
tree575a79d0c587ffe2a68b5905ec880ad6e4d9f6da
parent237bee37f87c2b2576944ca6a774503b9c9e2248 (diff)
parent66c607c6e8ab0f3fcf1351e198ae1ad4d947f83d (diff)
Merge "Enable global hacking checks and remove local checks"
-rw-r--r--HACKING.rst4
-rw-r--r--masakari/hacking/checks.py54
-rw-r--r--masakari/tests/unit/test_hacking.py88
-rw-r--r--tox.ini6
4 files changed, 6 insertions, 146 deletions
diff --git a/HACKING.rst b/HACKING.rst
index 374bc06..caec694 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -15,13 +15,10 @@ Masakari Specific Commandments
15 calls to datetime.datetime.utcnow() to make it easy to override its return value in tests 15 calls to datetime.datetime.utcnow() to make it easy to override its return value in tests
16- [M303] capitalize help string 16- [M303] capitalize help string
17 Config parameter help strings should have a capitalized first letter 17 Config parameter help strings should have a capitalized first letter
18- [M304] vim configuration should not be kept in source files.
19- [M305] Change assertTrue(isinstance(A, B)) by optimal assert like 18- [M305] Change assertTrue(isinstance(A, B)) by optimal assert like
20 assertIsInstance(A, B). 19 assertIsInstance(A, B).
21- [M306] Change assertEqual(type(A), B) by optimal assert like 20- [M306] Change assertEqual(type(A), B) by optimal assert like
22 assertIsInstance(A, B) 21 assertIsInstance(A, B)
23- [M307] Change assertEqual(A, None) or assertEqual(None, A) by optimal assert like
24 assertIsNone(A)
25- [M308] Validate that debug level logs are not translated. 22- [M308] Validate that debug level logs are not translated.
26- [M309] Don't import translation in tests 23- [M309] Don't import translation in tests
27- [M310] Setting CONF.* attributes directly in tests is forbidden. Use 24- [M310] Setting CONF.* attributes directly in tests is forbidden. Use
@@ -45,5 +42,4 @@ Masakari Specific Commandments
45- [M327] Python 3: do not use dict.iterkeys. 42- [M327] Python 3: do not use dict.iterkeys.
46- [M328] Python 3: do not use dict.itervalues. 43- [M328] Python 3: do not use dict.itervalues.
47- [M329] Deprecated library function os.popen() 44- [M329] Deprecated library function os.popen()
48- [M330] String interpolation should be delayed at logging calls.
49- [M331] LOG.warn is deprecated. Enforce use of LOG.warning. 45- [M331] LOG.warn is deprecated. Enforce use of LOG.warning.
diff --git a/masakari/hacking/checks.py b/masakari/hacking/checks.py
index 72c5449..baaac04 100644
--- a/masakari/hacking/checks.py
+++ b/masakari/hacking/checks.py
@@ -35,7 +35,6 @@ UNDERSCORE_IMPORT_FILES = []
35session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]") 35session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
36cfg_re = re.compile(r".*\scfg\.") 36cfg_re = re.compile(r".*\scfg\.")
37cfg_opt_re = re.compile(r".*[\s\[]cfg\.[a-zA-Z]*Opt\(") 37cfg_opt_re = re.compile(r".*[\s\[]cfg\.[a-zA-Z]*Opt\(")
38vi_header_re = re.compile(r"^#\s+vim?:.+")
39asse_trueinst_re = re.compile( 38asse_trueinst_re = re.compile(
40 r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, " 39 r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
41 "(\w|\.|\'|\"|\[|\])+\)\)") 40 "(\w|\.|\'|\"|\[|\])+\)\)")
@@ -86,9 +85,6 @@ spawn_re = re.compile(
86contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(") 85contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
87doubled_words_re = re.compile( 86doubled_words_re = re.compile(
88 r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b") 87 r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b")
89log_string_interpolation = re.compile(r".*LOG\.(error|warning|info"
90 r"|critical|exception|debug)"
91 r"\([^,]*%[^,]*[,)]")
92 88
93 89
94def no_db_session_in_public_api(logical_line, filename): 90def no_db_session_in_public_api(logical_line, filename):
@@ -124,20 +120,6 @@ def capital_cfg_help(logical_line, tokens):
124 yield(0, msg) 120 yield(0, msg)
125 121
126 122
127def no_vi_headers(physical_line, line_number, lines):
128 """Check for vi editor configuration in source files.
129
130 By default vi modelines can only appear in the first or
131 last 5 lines of a source file.
132
133 M304
134 """
135 # NOTE(abhishekk): line_number is 1-indexed
136 if line_number <= 5 or line_number > len(lines) - 5:
137 if vi_header_re.match(physical_line):
138 return 0, "M304: Don't put vi configuration in source files"
139
140
141def assert_true_instance(logical_line): 123def assert_true_instance(logical_line):
142 """Check for assertTrue(isinstance(a, b)) sentences 124 """Check for assertTrue(isinstance(a, b)) sentences
143 125
@@ -157,18 +139,6 @@ def assert_equal_type(logical_line):
157 yield (0, "M306: assertEqual(type(A), B) sentences not allowed") 139 yield (0, "M306: assertEqual(type(A), B) sentences not allowed")
158 140
159 141
160def assert_equal_none(logical_line):
161 """Check for assertEqual(A, None) or assertEqual(None, A) sentences
162
163 M307
164 """
165 res = (asse_equal_start_with_none_re.search(logical_line) or
166 asse_equal_end_with_none_re.search(logical_line))
167 if res:
168 yield (0, "M307: assertEqual(A, None) or assertEqual(None, A) "
169 "sentences not allowed")
170
171
172def no_translate_debug_logs(logical_line, filename): 142def no_translate_debug_logs(logical_line, filename):
173 """Check for 'LOG.debug(_(' 143 """Check for 'LOG.debug(_('
174 144
@@ -392,27 +362,6 @@ def no_os_popen(logical_line):
392 'Replace it using subprocess module. ') 362 'Replace it using subprocess module. ')
393 363
394 364
395def check_delayed_string_interpolation(logical_line, filename, noqa):
396 """M330 String interpolation should be delayed at logging calls.
397
398 M330: LOG.debug('Example: %s' % 'bad')
399 Okay: LOG.debug('Example: %s', 'good')
400 """
401 msg = ("M330 String interpolation should be delayed to be "
402 "handled by the logging code, rather than being done "
403 "at the point of the logging call. "
404 "Use ',' instead of '%'.")
405
406 if noqa:
407 return
408
409 if '/tests/' in filename:
410 return
411
412 if log_string_interpolation.match(logical_line):
413 yield(logical_line.index('%'), msg)
414
415
416def no_log_warn(logical_line): 365def no_log_warn(logical_line):
417 """Disallow 'LOG.warn(' 366 """Disallow 'LOG.warn('
418 367
@@ -431,11 +380,9 @@ def factory(register):
431 register(no_db_session_in_public_api) 380 register(no_db_session_in_public_api)
432 register(use_timeutils_utcnow) 381 register(use_timeutils_utcnow)
433 register(capital_cfg_help) 382 register(capital_cfg_help)
434 register(no_vi_headers)
435 register(no_import_translation_in_tests) 383 register(no_import_translation_in_tests)
436 register(assert_true_instance) 384 register(assert_true_instance)
437 register(assert_equal_type) 385 register(assert_equal_type)
438 register(assert_equal_none)
439 register(assert_raises_regexp) 386 register(assert_raises_regexp)
440 register(no_translate_debug_logs) 387 register(no_translate_debug_logs)
441 register(no_setting_conf_directly_in_tests) 388 register(no_setting_conf_directly_in_tests)
@@ -453,5 +400,4 @@ def factory(register):
453 register(check_python3_no_iterkeys) 400 register(check_python3_no_iterkeys)
454 register(check_python3_no_itervalues) 401 register(check_python3_no_itervalues)
455 register(no_os_popen) 402 register(no_os_popen)
456 register(check_delayed_string_interpolation)
457 register(no_log_warn) 403 register(no_log_warn)
diff --git a/masakari/tests/unit/test_hacking.py b/masakari/tests/unit/test_hacking.py
index aaff724..de52b97 100644
--- a/masakari/tests/unit/test_hacking.py
+++ b/masakari/tests/unit/test_hacking.py
@@ -53,30 +53,6 @@ class HackingTestCase(test.NoDBTestCase):
53 just assertTrue if the check is expected to fail and assertFalse if it 53 just assertTrue if the check is expected to fail and assertFalse if it
54 should pass. 54 should pass.
55 """ 55 """
56 def test_no_vi_headers(self):
57
58 lines = ['Line 1\n', 'Line 2\n', 'Line 3\n', 'Line 4\n', 'Line 5\n',
59 'Line 6\n', 'Line 7\n', 'Line 8\n', 'Line 9\n', 'Line 10\n',
60 'Line 11\n', 'Line 12\n', 'Line 13\n', 'Line14\n', 'Line15\n']
61
62 self.assertIsNone(checks.no_vi_headers(
63 "Test string foo", 1, lines))
64 self.assertEqual(len(list(checks.no_vi_headers(
65 "# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
66 2, lines))), 2)
67 self.assertIsNone(checks.no_vi_headers(
68 "# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
69 6, lines))
70 self.assertIsNone(checks.no_vi_headers(
71 "# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
72 9, lines))
73 self.assertEqual(len(list(checks.no_vi_headers(
74 "# vim: et tabstop=4 shiftwidth=4 softtabstop=4",
75 14, lines))), 2)
76 self.assertIsNone(checks.no_vi_headers(
77 "Test end string for vi",
78 15, lines))
79
80 def test_assert_true_instance(self): 56 def test_assert_true_instance(self):
81 self.assertEqual(len(list(checks.assert_true_instance( 57 self.assertEqual(len(list(checks.assert_true_instance(
82 "self.assertTrue(isinstance(e, " 58 "self.assertTrue(isinstance(e, "
@@ -129,19 +105,7 @@ class HackingTestCase(test.NoDBTestCase):
129 self.assertEqual(len(list(checks.assert_equal_in( 105 self.assertEqual(len(list(checks.assert_equal_in(
130 "self.assertEqual(False, any(a==1 for a in b))"))), 0) 106 "self.assertEqual(False, any(a==1 for a in b))"))), 0)
131 107
132 def test_assert_equal_none(self):
133 self.assertEqual(len(list(checks.assert_equal_none(
134 "self.assertEqual(A, None)"))), 1)
135
136 self.assertEqual(len(list(checks.assert_equal_none(
137 "self.assertEqual(None, A)"))), 1)
138
139 self.assertEqual(
140 len(list(checks.assert_equal_none("self.assertIsNone()"))), 0)
141
142 def test_assert_true_or_false_with_in_or_not_in(self): 108 def test_assert_true_or_false_with_in_or_not_in(self):
143 self.assertEqual(len(list(checks.assert_equal_none(
144 "self.assertEqual(A, None)"))), 1)
145 self.assertEqual(len(list(checks.assert_true_or_false_with_in( 109 self.assertEqual(len(list(checks.assert_true_or_false_with_in(
146 "self.assertTrue(A in B)"))), 1) 110 "self.assertTrue(A in B)"))), 1)
147 111
@@ -456,58 +420,6 @@ class HackingTestCase(test.NoDBTestCase):
456 self._assert_has_errors(code, checks.no_os_popen, 420 self._assert_has_errors(code, checks.no_os_popen,
457 expected_errors=errors) 421 expected_errors=errors)
458 422
459 def test_check_delayed_string_interpolation(self):
460 checker = checks.check_delayed_string_interpolation
461 code = """
462 msg_w = ('Test string (%s)')
463 msg_i = 'Test string (%s)'
464 value = 'test'
465
466 LOG.error(("Test string (%s)") % value)
467 LOG.warning(msg_w % 'test%string')
468 LOG.info(msg_i %
469 "test%string%info")
470 LOG.critical(
471 ('Test string (%s)') % value,
472 instance=instance)
473 LOG.exception((" 'Test quotation %s' \"Test\"") % 'test')
474 LOG.debug(' "Test quotation %s" \'Test\'' % "test")
475 LOG.debug('Tesing %(test)s' %
476 {'test': ','.join(
477 ['%s=%s' % (name, value)
478 for name, value in test.items()])})
479 """
480
481 expected_errors = [(5, 31, 'M330'), (6, 18, 'M330'), (7, 15, 'M330'),
482 (10, 25, 'M330'), (12, 46, 'M330'),
483 (13, 40, 'M330'), (14, 28, 'M330')]
484 self._assert_has_errors(code, checker, expected_errors=expected_errors)
485 self._assert_has_no_errors(
486 code, checker, filename='masakari/tests/unit/test_hacking.py')
487
488 code = """
489 msg_w = ('Test string (%s)')
490 msg_i = 'Test string (%s)'
491 value = 'test'
492
493 LOG.error(("Test string (%s)"), value)
494 LOG.error(("Test string (%s)") % value) # noqa
495 LOG.warn(('Test string (%s)'),
496 value)
497 LOG.info(msg_i,
498 "test%string%info")
499 LOG.critical(
500 ('Test string (%s)'), value,
501 instance=instance)
502 LOG.exception((" 'Test quotation %s' \"Test\""), 'test')
503 LOG.debug(' "Test quotation %s" \'Test\'', "test")
504 LOG.debug('Tesing %(test)s',
505 {'test': ','.join(
506 ['%s=%s' % (name, value)
507 for name, value in test.items()])})
508 """
509 self._assert_has_no_errors(code, checker)
510
511 def test_no_log_warn(self): 423 def test_no_log_warn(self):
512 code = """ 424 code = """
513 LOG.warn("LOG.warn is deprecated") 425 LOG.warn("LOG.warn is deprecated")
diff --git a/tox.ini b/tox.ini
index ae7083d..8cde388 100644
--- a/tox.ini
+++ b/tox.ini
@@ -61,6 +61,12 @@ commands = oslo_debug_helper {posargs}
61# E123, E125 skipped as they are invalid PEP-8. 61# E123, E125 skipped as they are invalid PEP-8.
62 62
63show-source = True 63show-source = True
64
65# The below hacking rules by default are disabled should be enabled:
66# [H106] Don't put vim configuration in source files.
67# [H203] Use assertIs(Not)None to check for None.
68# [H904] Delay string interpolations at logging calls.
69enable-extensions = H106,H203,H904
64ignore = E123,E125,E128,H405 70ignore = E123,E125,E128,H405
65builtins = _ 71builtins = _
66exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build 72exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build