Remove log translation related check
Since we're preparing to remove log translation from project, we should remove translation check to ensure tests pass. For log translation removal, see link [1][2]. [1] http://lists.openstack.org/pipermail/openstack-i18n/2016-November/002574.html [2] http://lists.openstack.org/pipermail/openstack-dev/2017-March/113365.html Change-Id: I7d3bd59cf95d9e10c2d578b63dc60313f1d09282
This commit is contained in:
Jeremy Liu
parent
b2db269be4
commit
3099cd8050
29
HACKING.rst
29
HACKING.rst
@ -12,11 +12,7 @@ Barbican Specific Commandments
|
||||
- [B310] Check for improper use of logging format arguments.
|
||||
- [B311] Use assertIsNone(...) instead of assertEqual(None, ...).
|
||||
- [B312] Use assertTrue(...) rather than assertEqual(True, ...).
|
||||
- [B313] Validate that debug level logs are not translated.
|
||||
- [B314] str() and unicode() cannot be used on an exception. Remove or use six.text_type().
|
||||
- [B315] Translated messages cannot be concatenated. String should be
|
||||
included in translated message.
|
||||
- [B316] Log messages, except debug ones, require translations!
|
||||
- [B317] 'oslo_' should be used instead of 'oslo.'
|
||||
- [B318] Must use a dict comprehension instead of a dict constructor
|
||||
with a sequence of key-value pairs.
|
||||
@ -25,31 +21,6 @@ Barbican Specific Commandments
|
||||
- [B321] Use assertIsNotNone(...) rather than assertNotEqual(None, ...) or
|
||||
assertIsNot(None, ...).
|
||||
|
||||
LOG Translations
|
||||
----------------
|
||||
|
||||
LOG.debug messages will not get translated. Use ``_LI()`` for
|
||||
``LOG.info``, ``_LW`` for ``LOG.warning``, ``_LE`` for ``LOG.error``
|
||||
and ``LOG.exception``, and ``_LC()`` for ``LOG.critical``.
|
||||
|
||||
``_()`` is preferred for any user facing message, even if it is also
|
||||
going to a log file. This ensures that the translated version of the
|
||||
message will be available to the user.
|
||||
|
||||
The log marker functions (``_LI()``, ``_LW()``, ``_LE()``, and ``_LC()``)
|
||||
must only be used when the message is only sent directly to the log.
|
||||
Anytime that the message will be passed outside of the current context
|
||||
(for example as part of an exception) the ``_()`` marker function
|
||||
must be used.
|
||||
|
||||
A common pattern is to define a single message object and use it more
|
||||
than once, for the log call and the exception. In that case, ``_()``
|
||||
must be used because the message is going to appear in an exception that
|
||||
may be presented to the user.
|
||||
|
||||
For more details about translations, see
|
||||
http://docs.openstack.org/developer/oslo.i18n/guidelines.html
|
||||
|
||||
Creating Unit Tests
|
||||
-------------------
|
||||
For every new feature, unit tests should be created that both test and
|
||||
|
||||
@ -34,26 +34,6 @@ Guidelines for writing new hacking checks
|
||||
|
||||
"""
|
||||
|
||||
UNDERSCORE_IMPORT_FILES = []
|
||||
|
||||
log_translation = re.compile(
|
||||
r"(.)*LOG\.(audit|error|info|critical|exception)\(\s*('|\")")
|
||||
log_translation_LC = re.compile(
|
||||
r"(.)*LOG\.(critical)\(\s*(_\(|'|\")")
|
||||
log_translation_LE = re.compile(
|
||||
r"(.)*LOG\.(error|exception)\(\s*(_\(|'|\")")
|
||||
log_translation_LI = re.compile(
|
||||
r"(.)*LOG\.(info)\(\s*(_\(|'|\")")
|
||||
log_translation_LW = re.compile(
|
||||
r"(.)*LOG\.(warning|warn)\(\s*(_\(|'|\")")
|
||||
translated_log = re.compile(
|
||||
r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)"
|
||||
"\(\s*_\(\s*('|\")")
|
||||
string_translation = re.compile(r"[^_]*_\(\s*('|\")")
|
||||
underscore_import_check = re.compile(r"(.)*import _$")
|
||||
underscore_import_check_multi = re.compile(r"(.)*import (.)*_, (.)*")
|
||||
# We need this for cases where they have created their own _ function.
|
||||
custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")
|
||||
oslo_namespace_imports = re.compile(r"from[\s]*oslo[.](.*)")
|
||||
dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
|
||||
assert_no_xrange_re = re.compile(r"\s*xrange\s*\(")
|
||||
@ -109,23 +89,6 @@ class BaseASTChecker(ast.NodeVisitor):
|
||||
return False
|
||||
|
||||
|
||||
def no_translate_debug_logs(logical_line, filename):
|
||||
"""Check for 'LOG.debug(u._('
|
||||
|
||||
As per our translation policy,
|
||||
https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
|
||||
we shouldn't translate debug level logs.
|
||||
|
||||
* This check assumes that 'LOG' is a logger.
|
||||
* Use filename so we can start enforcing this in specific folders instead
|
||||
of needing to do so all at once.
|
||||
|
||||
B313
|
||||
"""
|
||||
if logical_line.startswith("LOG.debug(u._("):
|
||||
yield(0, "B313 Don't translate debug level logs")
|
||||
|
||||
|
||||
class CheckLoggingFormatArgs(BaseASTChecker):
|
||||
"""Check for improper use of logging format arguments.
|
||||
|
||||
@ -191,30 +154,6 @@ class CheckLoggingFormatArgs(BaseASTChecker):
|
||||
return super(CheckLoggingFormatArgs, self).generic_visit(node)
|
||||
|
||||
|
||||
def validate_log_translations(logical_line, physical_line, filename):
|
||||
# Translations are not required in the test directories.
|
||||
if ("barbican/tests" in filename or "functional" in filename):
|
||||
return
|
||||
if pep8.noqa(physical_line):
|
||||
return
|
||||
msg = "B316: LOG.critical messages require translations `_LC()`!"
|
||||
if log_translation_LC.match(logical_line):
|
||||
yield (0, msg)
|
||||
msg = ("B316: LOG.error and LOG.exception messages require translations "
|
||||
"`_LE()`!")
|
||||
if log_translation_LE.match(logical_line):
|
||||
yield (0, msg)
|
||||
msg = "B316: LOG.info messages require translations `_LI()`!"
|
||||
if log_translation_LI.match(logical_line):
|
||||
yield (0, msg)
|
||||
msg = "B316: LOG.warning messages require translations `_LW()`!"
|
||||
if log_translation_LW.match(logical_line):
|
||||
yield (0, msg)
|
||||
msg = "B316: Log messages require translations!"
|
||||
if log_translation.match(logical_line):
|
||||
yield (0, msg)
|
||||
|
||||
|
||||
class CheckForStrUnicodeExc(BaseASTChecker):
|
||||
"""Checks for the use of str() or unicode() on an exception.
|
||||
|
||||
@ -262,28 +201,6 @@ class CheckForStrUnicodeExc(BaseASTChecker):
|
||||
super(CheckForStrUnicodeExc, self).generic_visit(node)
|
||||
|
||||
|
||||
class CheckForTransAdd(BaseASTChecker):
|
||||
"""Checks for the use of concatenation on a translated string.
|
||||
|
||||
Translations should not be concatenated with other strings, but
|
||||
should instead include the string being added to the translated
|
||||
string to give the translators the most information.
|
||||
"""
|
||||
|
||||
CHECK_DESC = ('B315 Translated messages cannot be concatenated. '
|
||||
'String should be included in translated message.')
|
||||
|
||||
TRANS_FUNC = ['_', '_LI', '_LW', '_LE', '_LC']
|
||||
|
||||
def visit_BinOp(self, node):
|
||||
if isinstance(node.op, ast.Add):
|
||||
if self._check_call_names(node.left, self.TRANS_FUNC):
|
||||
self.add_error(node.left)
|
||||
elif self._check_call_names(node.right, self.TRANS_FUNC):
|
||||
self.add_error(node.right)
|
||||
super(CheckForTransAdd, self).generic_visit(node)
|
||||
|
||||
|
||||
def check_oslo_namespace_imports(logical_line, physical_line, filename):
|
||||
"""'oslo_' should be used instead of 'oslo.'
|
||||
|
||||
@ -365,11 +282,8 @@ def validate_assertIsNotNone(logical_line):
|
||||
|
||||
|
||||
def factory(register):
|
||||
register(validate_log_translations)
|
||||
register(no_translate_debug_logs)
|
||||
register(CheckForStrUnicodeExc)
|
||||
register(CheckLoggingFormatArgs)
|
||||
register(CheckForTransAdd)
|
||||
register(check_oslo_namespace_imports)
|
||||
register(dict_constructor_with_list_copy)
|
||||
register(no_xrange)
|
||||
|
||||
@ -12,7 +12,6 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import sys
|
||||
import textwrap
|
||||
|
||||
import ddt
|
||||
@ -57,16 +56,6 @@ class HackingTestCase(utils.BaseTestCase):
|
||||
should pass.
|
||||
"""
|
||||
|
||||
def test_no_translate_debug_logs(self):
|
||||
self.assertEqual(1, len(list(checks.no_translate_debug_logs(
|
||||
"LOG.debug(u._('foo'))", "barbican/scheduler/foo.py"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.no_translate_debug_logs(
|
||||
"LOG.debug('foo')", "barbican/scheduler/foo.py"))))
|
||||
|
||||
self.assertEqual(0, len(list(checks.no_translate_debug_logs(
|
||||
"LOG.info(_('foo'))", "barbican/scheduler/foo.py"))))
|
||||
|
||||
# We are patching pep8 so that only the check under test is actually
|
||||
# installed.
|
||||
@mock.patch('pep8._checks',
|
||||
@ -187,50 +176,6 @@ class HackingTestCase(utils.BaseTestCase):
|
||||
errors = [(8, 20, 'B314'), (8, 33, 'B314'), (9, 16, 'B314')]
|
||||
self._assert_has_errors(code, checker, expected_errors=errors)
|
||||
|
||||
def test_trans_add(self):
|
||||
|
||||
checker = checks.CheckForTransAdd
|
||||
code = """
|
||||
def fake_tran(msg):
|
||||
return msg
|
||||
|
||||
|
||||
_ = fake_tran
|
||||
_LI = _
|
||||
_LW = _
|
||||
_LE = _
|
||||
_LC = _
|
||||
|
||||
|
||||
def f(a, b):
|
||||
msg = _('test') + 'add me'
|
||||
msg = _LI('test') + 'add me'
|
||||
msg = _LW('test') + 'add me'
|
||||
msg = _LE('test') + 'add me'
|
||||
msg = _LC('test') + 'add me'
|
||||
msg = 'add to me' + _('test')
|
||||
return msg
|
||||
"""
|
||||
|
||||
# Python 3.4.0 introduced a change to the column calculation during AST
|
||||
# parsing. This was reversed in Python 3.4.3, hence the version-based
|
||||
# expected value calculation. See #1499743 for more background.
|
||||
if sys.version_info < (3, 4, 0) or sys.version_info >= (3, 4, 3):
|
||||
errors = [(13, 10, 'B315'), (14, 10, 'B315'), (15, 10, 'B315'),
|
||||
(16, 10, 'B315'), (17, 10, 'B315'), (18, 24, 'B315')]
|
||||
else:
|
||||
errors = [(13, 11, 'B315'), (14, 13, 'B315'), (15, 13, 'B315'),
|
||||
(16, 13, 'B315'), (17, 13, 'B315'), (18, 25, 'B315')]
|
||||
self._assert_has_errors(code, checker, expected_errors=errors)
|
||||
|
||||
code = """
|
||||
def f(a, b):
|
||||
msg = 'test' + 'add me'
|
||||
return msg
|
||||
"""
|
||||
errors = []
|
||||
self._assert_has_errors(code, checker, expected_errors=errors)
|
||||
|
||||
def test_dict_constructor_with_list_copy(self):
|
||||
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
|
||||
" dict([(i, connect_info[i])"))))
|
||||
|
||||
Loading…
Reference in New Issue
Block a user