From: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Date: Tue, 25 Nov 2014 04:53:02 +0000 (+0900)
Subject: hacking: Check if correct log markers are used
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fda6226a345c709da3facc464826344b6b05e994;p=openstack-build%2Fneutron-build.git

hacking: Check if correct log markers are used

Makes the check tighter and would detect mistakes
like LOG.info(_LE("foo")).

This would reduce reviewer loads for relevant changes.

Partial-Bug: #1320867
Change-Id: I66c7ab1fd9b40beb857dc6c4b143ca47a5ebce4b
---

diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py
index 8474ee533..9a7599dbc 100644
--- a/neutron/hacking/checks.py
+++ b/neutron/hacking/checks.py
@@ -35,9 +35,24 @@ log_translation = re.compile(
     r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)\(\s*('|\")")
 author_tag_re = (re.compile("^\s*#\s*@?(a|A)uthor"),
                  re.compile("^\.\.\s+moduleauthor::"))
-log_translation_hint = re.compile(
-    r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)"
-    "\(\s*(_\(|'|\")")
+_all_hints = set(['_', '_LI', '_LE', '_LW', '_LC'])
+_all_log_levels = {
+    # NOTE(yamamoto): Following nova which uses _() for audit.
+    'audit': '_',
+    'error': '_LE',
+    'info': '_LI',
+    'warn': '_LW',
+    'warning': '_LW',
+    'critical': '_LC',
+    'exception': '_LE',
+}
+log_translation_hints = []
+for level, hint in _all_log_levels.iteritems():
+    r = "(.)*LOG\.%(level)s\(\s*((%(wrong_hints)s)\(|'|\")" % {
+        'level': level,
+        'wrong_hints': '|'.join(_all_hints - set([hint])),
+    }
+    log_translation_hints.append(re.compile(r))
 
 
 def _directory_to_check_translation(filename):
@@ -77,8 +92,9 @@ def validate_log_translations(logical_line, physical_line, filename):
 
     if _directory_to_check_translation(filename):
         msg = "N320: Log messages require translation hints!"
-        if log_translation_hint.match(logical_line):
-            yield (0, msg)
+        for log_translation_hint in log_translation_hints:
+            if log_translation_hint.match(logical_line):
+                yield (0, msg)
 
 
 def use_jsonutils(logical_line, filename):
diff --git a/neutron/tests/unit/test_hacking.py b/neutron/tests/unit/test_hacking.py
index 0a8fa0de6..d82f5febe 100644
--- a/neutron/tests/unit/test_hacking.py
+++ b/neutron/tests/unit/test_hacking.py
@@ -17,9 +17,18 @@ from neutron.tests import base
 class HackingTestCase(base.BaseTestCase):
 
     def test_log_translations(self):
-        logs = ['audit', 'error', 'info', 'warn', 'warning', 'critical',
-                'exception']
+        expected_marks = {
+            'audit': '_',
+            'error': '_LE',
+            'info': '_LI',
+            'warn': '_LW',
+            'warning': '_LW',
+            'critical': '_LC',
+            'exception': '_LE',
+        }
+        logs = expected_marks.keys()
         levels = ['_LI', '_LW', '_LE', '_LC']
+        all_marks = levels + ['_']
         debug = "LOG.debug('OK')"
         self.assertEqual(
             0, len(list(checks.validate_log_translations(debug, debug, 'f'))))
@@ -42,11 +51,13 @@ class HackingTestCase(base.BaseTestCase):
                     0, len(list(checks.validate_log_translations(ok,
                                                                  ok, 'f'))))
             filename = 'neutron/agent/f'
-            bad = "LOG.%s(_('BAD - by directory'))" % log
-            self.assertEqual(
-                1, len(list(checks.validate_log_translations(bad,
-                                                             bad,
-                                                             filename))))
+            for mark in all_marks:
+                stmt = "LOG.%s(%s('test'))" % (log, mark)
+                self.assertEqual(
+                    0 if expected_marks[log] == mark else 1,
+                    len(list(checks.validate_log_translations(stmt,
+                                                              stmt,
+                                                              filename))))
 
     def test_use_jsonutils(self):
         def __get_msg(fun):