From 7539e4510b78d8622ff466d891b14db61546df42 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Cuong Date: Tue, 4 Jul 2017 03:50:54 -0400 Subject: [PATCH] Update log translation hacking rule Starting with the Pike series, OpenStack no longer supports log translation. Update hacking rule to prevent log translation in all log level instead of only debug level. Change-Id: I00d1930ba2bb0e0278e0d33147e4d15c59615e74 --- glare/hacking/checks.py | 38 ++++++++++++-------------------- glare/tests/unit/test_hacking.py | 16 ++++++-------- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/glare/hacking/checks.py b/glare/hacking/checks.py index ef42836..38b3f70 100644 --- a/glare/hacking/checks.py +++ b/glare/hacking/checks.py @@ -41,18 +41,13 @@ asse_equal_end_with_none_re = re.compile( asse_equal_start_with_none_re = re.compile( r"(.)*assertEqual\(None, (\w|\.|\'|\"|\[|\])+\)") unicode_func_re = re.compile(r"(\s|\W|^)unicode\(") -log_translation = re.compile( - r"(.)*LOG\.(audit)\(\s*('|\")") -log_translation_info = re.compile( - r"(.)*LOG\.(info)\(\s*(_\(|'|\")") -log_translation_exception = re.compile( - r"(.)*LOG\.(exception)\(\s*(_\(|'|\")") -log_translation_error = re.compile( - r"(.)*LOG\.(error)\(\s*(_\(|'|\")") -log_translation_critical = re.compile( - r"(.)*LOG\.(critical)\(\s*(_\(|'|\")") -log_translation_warning = re.compile( - r"(.)*LOG\.(warning)\(\s*(_\(|'|\")") + +_all_log_levels = {'debug', 'error', 'info', 'warning', + 'critical', 'exception'} +# Since _Lx have been removed, we just need to check _() +translated_logs = re.compile( + r"(.)*LOG\.(%(level)s)\(\s*_\(" % {'level': '|'.join(_all_log_levels)}) + dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") @@ -86,18 +81,13 @@ def assert_equal_none(logical_line): "sentences not allowed") -def no_translate_debug_logs(logical_line, filename): - dirs = [ - "glare/api", - "glare/cmd", - "glare/common", - "glare/db", - "glare/tests", - ] +def no_translate_logs(logical_line): + """Check for use of LOG.*(_( - if max([name in filename for name in dirs]): - if logical_line.startswith("LOG.debug(_("): - yield(0, "G319: Don't translate debug level logs") + G319 + """ + if translated_logs.match(logical_line): + yield (0, "G319: Don't translate logs") def no_direct_use_of_unicode_function(logical_line): @@ -156,7 +146,7 @@ def factory(register): register(assert_true_instance) register(assert_equal_type) register(assert_equal_none) - register(no_translate_debug_logs) + register(no_translate_logs) register(no_direct_use_of_unicode_function) register(check_no_contextlib_nested) register(dict_constructor_with_list_copy) diff --git a/glare/tests/unit/test_hacking.py b/glare/tests/unit/test_hacking.py index 6c04125..6eca789 100644 --- a/glare/tests/unit/test_hacking.py +++ b/glare/tests/unit/test_hacking.py @@ -44,15 +44,13 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual( 0, len(list(checks.assert_equal_none("self.assertIsNone()")))) - def test_no_translate_debug_logs(self): - self.assertEqual(1, len(list(checks.no_translate_debug_logs( - "LOG.debug(_('foo'))", "glare/common/foo.py")))) - - self.assertEqual(0, len(list(checks.no_translate_debug_logs( - "LOG.debug('foo')", "glare/common/foo.py")))) - - self.assertEqual(0, len(list(checks.no_translate_debug_logs( - "LOG.info(_('foo'))", "glare/common/foo.py")))) + def test_no_translate_logs(self): + for log in checks._all_log_levels: + bad = 'LOG.%s(_("Bad"))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad)))) + # Catch abuses when used with a variable and not a literal + bad = 'LOG.%s(_(msg))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad)))) def test_no_direct_use_of_unicode_function(self): self.assertEqual(1, len(list(checks.no_direct_use_of_unicode_function(