add hacking check for config options location
This adds a hackign check which raises a pep8 error if a config option was found in location where it shouldn't be. Change-Id: I75df3404cc6178aa179e96d75f154e389c3008af
This commit is contained in:
@@ -52,6 +52,7 @@ Nova Specific Commandments
|
|||||||
- [N339] Check common raise_feature_not_supported() is used for v2.1 HTTPNotImplemented response.
|
- [N339] Check common raise_feature_not_supported() is used for v2.1 HTTPNotImplemented response.
|
||||||
- [N340] Check nova.utils.spawn() is used instead of greenthread.spawn() and eventlet.spawn()
|
- [N340] Check nova.utils.spawn() is used instead of greenthread.spawn() and eventlet.spawn()
|
||||||
- [N341] contextlib.nested is deprecated
|
- [N341] contextlib.nested is deprecated
|
||||||
|
- [N342] Config options should be in the central location ``nova/conf/``
|
||||||
|
|
||||||
Creating Unit Tests
|
Creating Unit Tests
|
||||||
-------------------
|
-------------------
|
||||||
|
@@ -36,6 +36,8 @@ UNDERSCORE_IMPORT_FILES = []
|
|||||||
|
|
||||||
session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
|
session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
|
||||||
cfg_re = re.compile(r".*\scfg\.")
|
cfg_re = re.compile(r".*\scfg\.")
|
||||||
|
# Excludes oslo.config OptGroup objects
|
||||||
|
cfg_opt_re = re.compile(r".*[\s\[]cfg\.[a-zA-Z]*Opt\(")
|
||||||
vi_header_re = re.compile(r"^#\s+vim?:.+")
|
vi_header_re = re.compile(r"^#\s+vim?:.+")
|
||||||
virt_file_re = re.compile(r"\./nova/(?:tests/)?virt/(\w+)/")
|
virt_file_re = re.compile(r"\./nova/(?:tests/)?virt/(\w+)/")
|
||||||
virt_import_re = re.compile(
|
virt_import_re = re.compile(
|
||||||
@@ -550,6 +552,30 @@ def check_no_contextlib_nested(logical_line, filename):
|
|||||||
yield(0, msg)
|
yield(0, msg)
|
||||||
|
|
||||||
|
|
||||||
|
def check_config_option_in_central_place(logical_line, filename):
|
||||||
|
msg = ("N342: Config options should be in the central location "
|
||||||
|
"'/nova/conf/*'. Do not declare new config options outside "
|
||||||
|
"of that folder.")
|
||||||
|
# That's the correct location
|
||||||
|
if "nova/conf/" in filename:
|
||||||
|
return
|
||||||
|
# TODO(markus_z) This is just temporary until all config options are
|
||||||
|
# moved to the central place. To avoid that a once cleaned up place
|
||||||
|
# introduces new config options, we do a check here. This array will
|
||||||
|
# get quite huge over the time, but will be removed at the end of the
|
||||||
|
# reorganization.
|
||||||
|
# You can add the full path to a module or folder. It's just a substring
|
||||||
|
# check, which makes it flexible enough.
|
||||||
|
cleaned_up = ["nova/console/serial.py",
|
||||||
|
"nova/cmd/serialproxy.py",
|
||||||
|
]
|
||||||
|
if not any(c in filename for c in cleaned_up):
|
||||||
|
return
|
||||||
|
|
||||||
|
if cfg_opt_re.match(logical_line):
|
||||||
|
yield(0, msg)
|
||||||
|
|
||||||
|
|
||||||
def factory(register):
|
def factory(register):
|
||||||
register(import_no_db_in_virt)
|
register(import_no_db_in_virt)
|
||||||
register(no_db_session_in_public_api)
|
register(no_db_session_in_public_api)
|
||||||
@@ -578,3 +604,4 @@ def factory(register):
|
|||||||
register(check_http_not_implemented)
|
register(check_http_not_implemented)
|
||||||
register(check_no_contextlib_nested)
|
register(check_no_contextlib_nested)
|
||||||
register(check_greenthread_spawns)
|
register(check_greenthread_spawns)
|
||||||
|
register(check_config_option_in_central_place)
|
||||||
|
@@ -588,3 +588,43 @@ class HackingTestCase(test.NoDBTestCase):
|
|||||||
|
|
||||||
code = "nova.utils.spawn_n(func, arg1, kwarg1=kwarg1)"
|
code = "nova.utils.spawn_n(func, arg1, kwarg1=kwarg1)"
|
||||||
self._assert_has_no_errors(code, checks.check_greenthread_spawns)
|
self._assert_has_no_errors(code, checks.check_greenthread_spawns)
|
||||||
|
|
||||||
|
def test_config_option_regex_match(self):
|
||||||
|
def should_match(code):
|
||||||
|
self.assertTrue(checks.cfg_opt_re.match(code))
|
||||||
|
|
||||||
|
def should_not_match(code):
|
||||||
|
self.assertFalse(checks.cfg_opt_re.match(code))
|
||||||
|
|
||||||
|
should_match("opt = cfg.StrOpt('opt_name')")
|
||||||
|
should_match("opt = cfg.IntOpt('opt_name')")
|
||||||
|
should_match("opt = cfg.DictOpt('opt_name')")
|
||||||
|
should_match("opt = cfg.Opt('opt_name')")
|
||||||
|
should_match("opts=[cfg.Opt('opt_name')]")
|
||||||
|
should_match(" cfg.Opt('opt_name')")
|
||||||
|
should_not_match("opt_group = cfg.OptGroup('opt_group_name')")
|
||||||
|
|
||||||
|
def test_check_config_option_in_central_place(self):
|
||||||
|
errors = [(1, 0, "N342")]
|
||||||
|
code = """
|
||||||
|
opts = [
|
||||||
|
cfg.StrOpt('random_opt',
|
||||||
|
default='foo',
|
||||||
|
help='I am here to do stuff'),
|
||||||
|
]
|
||||||
|
"""
|
||||||
|
# option at the right place in the tree
|
||||||
|
self._assert_has_no_errors(code,
|
||||||
|
checks.check_config_option_in_central_place,
|
||||||
|
filename="nova/conf/serial_console.py")
|
||||||
|
# option at a location which is not in scope right now
|
||||||
|
# TODO(markus_z): This is remporary until all config options are
|
||||||
|
# moved to /nova/conf
|
||||||
|
self._assert_has_no_errors(code,
|
||||||
|
checks.check_config_option_in_central_place,
|
||||||
|
filename="nova/dummy/non_existent.py")
|
||||||
|
# option at the wrong place in the tree
|
||||||
|
self._assert_has_errors(code,
|
||||||
|
checks.check_config_option_in_central_place,
|
||||||
|
filename="nova/cmd/serialproxy.py",
|
||||||
|
expected_errors=errors)
|
||||||
|
Reference in New Issue
Block a user