diff --git a/HACKING.rst b/HACKING.rst new file mode 100644 index 000000000..72668ae7e --- /dev/null +++ b/HACKING.rst @@ -0,0 +1,11 @@ +Ceilometer Style Commandments +============================= + +- Step 1: Read the OpenStack Style Commandments + http://docs.openstack.org/developer/hacking/ +- Step 2: Read on + +Ceilometer Specific Commandments +-------------------------------- + +- [C300] Check for oslo library imports use the non-namespaced packages diff --git a/ceilometer/event/storage/impl_elasticsearch.py b/ceilometer/event/storage/impl_elasticsearch.py index 4e895ba0a..c594c266e 100644 --- a/ceilometer/event/storage/impl_elasticsearch.py +++ b/ceilometer/event/storage/impl_elasticsearch.py @@ -16,8 +16,8 @@ import operator import elasticsearch as es from elasticsearch import helpers -from oslo.utils import netutils -from oslo.utils import timeutils +from oslo_utils import netutils +from oslo_utils import timeutils import six from ceilometer.event.storage import base diff --git a/ceilometer/hacking/__init__.py b/ceilometer/hacking/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ceilometer/hacking/checks.py b/ceilometer/hacking/checks.py new file mode 100644 index 000000000..d09bc9b95 --- /dev/null +++ b/ceilometer/hacking/checks.py @@ -0,0 +1,51 @@ +# Copyright 2015 Huawei Technologies Co., Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Guidelines for writing new hacking checks + + - Use only for Ceilometer specific tests. OpenStack general tests + should be submitted to the common 'hacking' module. + - Pick numbers in the range C3xx. Find the current test with + the highest allocated number and then pick the next value. + - Keep the test method code in the source file ordered based + on the C3xx value. + - List the new rule in the top level HACKING.rst file + - Add test cases for each new rule to ceilometer/tests/test_hacking.py + +""" + +import re + + +# TODO(zqfan): When other oslo libraries switch over non-namespace'd +# imports, we need to add them to the regexp below. +oslo_namespace_imports = re.compile( + r"(from|import) oslo[.](concurrency|config|utils|i18n|serialization)") + + +def check_oslo_namespace_imports(logical_line, physical_line, filename): + # ignore openstack.common since they are not maintained by us + if 'ceilometer/openstack/common/' in filename: + return + + if re.match(oslo_namespace_imports, logical_line): + msg = ("C300: '%s' must be used instead of '%s'." % ( + logical_line.replace('oslo.', 'oslo_'), + logical_line)) + yield(0, msg) + + +def factory(register): + register(check_oslo_namespace_imports) diff --git a/ceilometer/publisher/direct.py b/ceilometer/publisher/direct.py index 14180ae76..220b36de7 100644 --- a/ceilometer/publisher/direct.py +++ b/ceilometer/publisher/direct.py @@ -13,8 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo.config import cfg -from oslo.utils import timeutils +from oslo_config import cfg +from oslo_utils import timeutils from ceilometer.dispatcher import database from ceilometer import publisher diff --git a/ceilometer/tests/compute/pollsters/test_instance.py b/ceilometer/tests/compute/pollsters/test_instance.py index 8c167b43e..eaa144d22 100644 --- a/ceilometer/tests/compute/pollsters/test_instance.py +++ b/ceilometer/tests/compute/pollsters/test_instance.py @@ -15,7 +15,7 @@ # under the License. import mock -from oslo.config import fixture as fixture_config +from oslo_config import fixture as fixture_config from ceilometer.agent import manager from ceilometer.compute.pollsters import instance as pollsters_instance diff --git a/ceilometer/tests/gabbi/fixtures.py b/ceilometer/tests/gabbi/fixtures.py index a9c9d38a6..204cd3d22 100644 --- a/ceilometer/tests/gabbi/fixtures.py +++ b/ceilometer/tests/gabbi/fixtures.py @@ -24,7 +24,7 @@ from unittest import case import uuid from gabbi import fixture -from oslo.config import fixture as fixture_config +from oslo_config import fixture as fixture_config from ceilometer.publisher import utils from ceilometer import sample diff --git a/ceilometer/tests/publisher/test_direct.py b/ceilometer/tests/publisher/test_direct.py index e8d7cdfa0..cfdcb4926 100644 --- a/ceilometer/tests/publisher/test_direct.py +++ b/ceilometer/tests/publisher/test_direct.py @@ -18,7 +18,7 @@ import datetime import uuid -from oslo.utils import netutils +from oslo_utils import netutils from ceilometer.event.storage import models as event from ceilometer.publisher import direct diff --git a/ceilometer/tests/test_hacking.py b/ceilometer/tests/test_hacking.py new file mode 100644 index 000000000..dea08ca37 --- /dev/null +++ b/ceilometer/tests/test_hacking.py @@ -0,0 +1,90 @@ +# Copyright 2015 Huawei Technologies Co., Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import textwrap + +import mock +import pep8 +from testtools import testcase + +from ceilometer.hacking import checks + + +class HackingTestCase(testcase.TestCase): + """Test cases for ceilometer specific hacking rules. + + This class tests the hacking checks in ceilometer.hacking.checks by + passing strings to the check methods like the pep8/flake8 parser would. + The parser loops over each line in the file and then passes the parameters + to the check method. The parameter names in the check method dictate what + type of object is passed to the check method. The parameter types are:: + + logical_line: A processed line with the following modifications: + - Multi-line statements converted to a single line. + - Stripped left and right. + - Contents of strings replaced with "xxx" of same length. + - Comments removed. + physical_line: Raw line of text from the input file. + lines: a list of the raw lines from the input file + tokens: the tokens that contribute to this logical line + line_number: line number in the input file + total_lines: number of lines in the input file + blank_lines: blank lines before this one + indent_char: indentation character in this file (" " or "\t") + indent_level: indentation (with tabs expanded to multiples of 8) + previous_indent_level: indentation on previous line + previous_logical: previous logical line + filename: Path of the file being run through pep8 + + When running a test on a check method the return will be False/None if + there is no violation in the sample input. If there is an error a tuple is + returned with a position in the line, and a message. So to check the result + just assertTrue if the check is expected to fail and assertFalse if it + should pass. + """ + + # We are patching pep8 so that only the check under test is actually + # installed. + @mock.patch('pep8._checks', + {'physical_line': {}, 'logical_line': {}, 'tree': {}}) + def _run_check(self, code, checker, filename=None): + pep8.register_check(checker) + + lines = textwrap.dedent(code).strip().splitlines(True) + + checker = pep8.Checker(filename=filename, lines=lines) + checker.check_all() + checker.report._deferred_print.sort() + return checker.report._deferred_print + + def _assert_has_errors(self, code, checker, expected_errors=None, + filename=None): + actual_errors = [e[:3] for e in + self._run_check(code, checker, filename)] + self.assertEqual(expected_errors or [], actual_errors) + + def test_oslo_namespace_imports_check(self): + codes = [ + "from oslo.concurrency import processutils", + "from oslo.config import cfg", + "import oslo.i18n", + "from oslo.utils import timeutils", + "from oslo.serialization import jsonutils", + ] + for code in codes: + self._assert_has_errors(code, checks.check_oslo_namespace_imports, + expected_errors=[(1, 0, "C300")]) + self._assert_has_errors( + code, checks.check_oslo_namespace_imports, + filename="ceilometer/openstack/common/xyz.py") diff --git a/tox.ini b/tox.ini index 2e452e82e..e88165fa0 100644 --- a/tox.ini +++ b/tox.ini @@ -98,3 +98,4 @@ show-source = True [hacking] import_exceptions = ceilometer.i18n +local-check-factory = ceilometer.hacking.checks.factory