diff --git a/.coveragerc b/.coveragerc index 3be721b..3d4697d 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,6 +1,7 @@ [run] branch = True source = octaviaclient +omit = octaviaclient/tests/* [report] ignore_errors = True diff --git a/octaviaclient/hacking/checks.py b/octaviaclient/hacking/checks.py index 47c5399..f1ec081 100644 --- a/octaviaclient/hacking/checks.py +++ b/octaviaclient/hacking/checks.py @@ -30,9 +30,6 @@ Guidelines for writing new hacking checks """ -author_tag_re = (re.compile("^\s*#\s*@?(a|A)uthor"), - re.compile("^\.\.\s+moduleauthor::")) - _all_log_levels = {'critical', 'error', 'exception', 'info', 'warning'} _all_hints = {'_LC', '_LE', '_LI', '_', '_LW'} @@ -95,16 +92,6 @@ def assert_equal_or_not_none(logical_line): yield (0, msg) -def no_author_tags(physical_line): - for regex in author_tag_re: - if regex.match(physical_line): - physical_line = physical_line.lower() - pos = physical_line.find('moduleauthor') - if pos < 0: - pos = physical_line.find('author') - return pos, "O322: Don't use author tags" - - def assert_equal_true_or_false(logical_line): """Check for assertEqual(True, A) or assertEqual(False, A) sentences @@ -272,7 +259,6 @@ def factory(register): register(assert_true_instance) register(assert_equal_or_not_none) register(no_translate_logs) - register(no_author_tags) register(assert_equal_true_or_false) register(no_mutable_default_args) register(assert_equal_in) diff --git a/octaviaclient/tests/unit/test_hacking.py b/octaviaclient/tests/unit/test_hacking.py index 0618dd5..064e4fd 100644 --- a/octaviaclient/tests/unit/test_hacking.py +++ b/octaviaclient/tests/unit/test_hacking.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +import testtools + from oslotest import base from octaviaclient.hacking import checks @@ -50,6 +52,13 @@ class HackingTestCase(base.BaseTestCase): should pass. """ + def assertLinePasses(self, func, *args): + with testtools.ExpectedException(StopIteration): + next(func(*args)) + + 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, " @@ -79,6 +88,14 @@ class HackingTestCase(base.BaseTestCase): 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):")))) + self.assertEqual(1, len(list(checks.no_mutable_default_args( + "def foo (bar=[]):")))) + 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)")))) @@ -143,9 +160,77 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual(0, len(list(checks.no_xrange( "range(45)")))) + def test_no_log_translations(self): + for log in checks._all_log_levels: + for hint in checks._all_hints: + bad = 'LOG.%s(%s("Bad"))' % (log, hint) + self.assertEqual( + 1, len(list(checks.no_translate_logs(bad, 'f')))) + # Catch abuses when used with a variable and not a literal + bad = 'LOG.%s(%s(msg))' % (log, hint) + self.assertEqual( + 1, len(list(checks.no_translate_logs(bad, 'f')))) + # Do not do validations in tests + ok = 'LOG.%s(_("OK - unit tests"))' % log + self.assertEqual( + 0, len(list(checks.no_translate_logs(ok, 'f/tests/f')))) + + def test_check_localized_exception_messages(self): + f = checks.check_raised_localized_exceptions + self.assertLineFails(f, " raise KeyError('Error text')", '') + self.assertLineFails(f, ' raise KeyError("Error text")', '') + self.assertLinePasses(f, ' raise KeyError(_("Error text"))', '') + self.assertLinePasses(f, ' raise KeyError(_ERR("Error text"))', '') + self.assertLinePasses(f, " raise KeyError(translated_msg)", '') + self.assertLinePasses(f, '# raise KeyError("Not translated")', '') + self.assertLinePasses(f, 'print("raise KeyError("Not ' + 'translated")")', '') + + def test_check_localized_exception_message_skip_tests(self): + f = checks.check_raised_localized_exceptions + self.assertLinePasses(f, "raise KeyError('Error text')", + 'neutron_lib/tests/unit/mytest.py') + + def test_check_no_basestring(self): + self.assertEqual(1, len(list(checks.check_no_basestring( + "isinstance('foo', basestring)")))) + + self.assertEqual(0, len(list(checks.check_no_basestring( + "isinstance('foo', six.string_types)")))) + + def test_dict_iteritems(self): + self.assertEqual(1, len(list(checks.check_python3_no_iteritems( + "obj.iteritems()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "six.iteritems(obj)")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "obj.items()")))) + + def test_check_no_eventlet_imports(self): + f = checks.check_no_eventlet_imports + self.assertLinePasses(f, 'from not_eventlet import greenthread') + self.assertLineFails(f, 'from eventlet import greenthread') + self.assertLineFails(f, 'import eventlet') + def test_line_continuation_no_backslash(self): results = list(checks.check_line_continuation_no_backslash( '', [(1, 'import', (2, 0), (2, 6), 'import \\\n'), (1, 'os', (3, 4), (3, 6), ' os\n')])) self.assertEqual(1, len(results)) self.assertEqual((2, 7), results[0][0]) + + def _get_factory_checks(self, factory): + check_fns = [] + + def _reg(check_fn): + self.assertTrue(hasattr(check_fn, '__call__')) + self.assertFalse(check_fn in check_fns) + check_fns.append(check_fn) + + factory(_reg) + return check_fns + + def test_factory(self): + self.assertTrue(len(self._get_factory_checks(checks.factory)) > 0) diff --git a/tox.ini b/tox.ini index 5fca5b6..e696453 100644 --- a/tox.ini +++ b/tox.ini @@ -42,6 +42,7 @@ commands = coverage combine coverage html -d cover coverage xml -o cover/coverage.xml + coverage report --fail-under=90 --skip-covered [testenv:docs] basepython = python3