Add hacking rules & fix hacking issues
Change-Id: Id335bfdf49ed88edd14e896a48c22b11113600c8
This commit is contained in:
Boris Pavlovic
parent
cabf79d44c
commit
435e6a6fc3
|
|
@ -24,8 +24,8 @@ utils.import_modules_from_package("osprofiler._notifiers")
|
|||
|
||||
_conf = configparser.ConfigParser()
|
||||
_conf.read(os.path.join(
|
||||
os.path.dirname(os.path.dirname(__file__)), 'setup.cfg'))
|
||||
os.path.dirname(os.path.dirname(__file__)), "setup.cfg"))
|
||||
try:
|
||||
__version__ = _conf.get('metadata', 'version')
|
||||
__version__ = _conf.get("metadata", "version")
|
||||
except (configparser.NoOptionError, configparser.NoSectionError):
|
||||
__version__ = None
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ def env(*args, **kwargs):
|
|||
value = os.environ.get(arg)
|
||||
if value:
|
||||
return value
|
||||
return kwargs.get('default', '')
|
||||
return kwargs.get("default", "")
|
||||
|
||||
|
||||
def arg(*args, **kwargs):
|
||||
|
|
@ -46,7 +46,7 @@ def arg(*args, **kwargs):
|
|||
def add_arg(func, *args, **kwargs):
|
||||
"""Bind CLI arguments to a shell.py `do_foo` function."""
|
||||
|
||||
if not hasattr(func, 'arguments'):
|
||||
if not hasattr(func, "arguments"):
|
||||
func.arguments = []
|
||||
|
||||
# NOTE(sirp): avoid dups that can occur when the module is shared across
|
||||
|
|
|
|||
|
|
@ -28,12 +28,12 @@ class BaseCommand(object):
|
|||
class TraceCommands(BaseCommand):
|
||||
group_name = "trace"
|
||||
|
||||
@cliutils.arg('trace_id', help='trace id')
|
||||
@cliutils.arg('--json', dest='use_json', action='store_true',
|
||||
help='show trace in JSON')
|
||||
@cliutils.arg('--html', dest='use_html', action='store_true',
|
||||
help='show trace in HTML')
|
||||
@cliutils.arg('--out', dest='file_name', help='save output in file')
|
||||
@cliutils.arg("trace_id", help="trace id")
|
||||
@cliutils.arg("--json", dest="use_json", action="store_true",
|
||||
help="show trace in JSON")
|
||||
@cliutils.arg("--html", dest="use_html", action="store_true",
|
||||
help="show trace in HTML")
|
||||
@cliutils.arg("--out", dest="file_name", help="save output in file")
|
||||
def show(self, args):
|
||||
"""Displays trace-results by given trace id in HTML or JSON format."""
|
||||
try:
|
||||
|
|
@ -50,7 +50,7 @@ class TraceCommands(BaseCommand):
|
|||
notifications = ceiloparser.get_notifications(
|
||||
client, args.trace_id)
|
||||
except Exception as e:
|
||||
if hasattr(e, 'http_status') and e.http_status == 401:
|
||||
if hasattr(e, "http_status") and e.http_status == 401:
|
||||
msg = "Invalid OpenStack Identity credentials."
|
||||
else:
|
||||
msg = "Something has gone wrong. See logs for more details."
|
||||
|
|
@ -80,7 +80,7 @@ class TraceCommands(BaseCommand):
|
|||
"output-formats: --json or --html.")
|
||||
|
||||
if args.file_name:
|
||||
with open(args.file_name, 'w+') as output_file:
|
||||
with open(args.file_name, "w+") as output_file:
|
||||
output_file.write(output)
|
||||
else:
|
||||
print (output)
|
||||
|
|
|
|||
|
|
@ -68,8 +68,8 @@ class OSProfilerShell(object):
|
|||
add_help=True
|
||||
)
|
||||
|
||||
parser.add_argument('-v', '--version',
|
||||
action='version',
|
||||
parser.add_argument("-v", "--version",
|
||||
action="version",
|
||||
version=osprofiler.__version__)
|
||||
|
||||
self._append_ceilometer_args(parser)
|
||||
|
|
@ -79,24 +79,24 @@ class OSProfilerShell(object):
|
|||
return parser
|
||||
|
||||
def _append_ceilometer_args(self, parent_parser):
|
||||
parser = parent_parser.add_argument_group('ceilometer')
|
||||
parser = parent_parser.add_argument_group("ceilometer")
|
||||
parser.add_argument(
|
||||
'--ceilometer-url', default=cliutils.env('CEILOMETER_URL'),
|
||||
help='Defaults to env[CEILOMETER_URL].')
|
||||
"--ceilometer-url", default=cliutils.env("CEILOMETER_URL"),
|
||||
help="Defaults to env[CEILOMETER_URL].")
|
||||
parser.add_argument(
|
||||
'--ceilometer-api-version',
|
||||
default=cliutils.env('CEILOMETER_API_VERSION', default='2'),
|
||||
help='Defaults to env[CEILOMETER_API_VERSION] or 2.')
|
||||
"--ceilometer-api-version",
|
||||
default=cliutils.env("CEILOMETER_API_VERSION", default="2"),
|
||||
help="Defaults to env[CEILOMETER_API_VERSION] or 2.")
|
||||
|
||||
def _append_identity_args(self, parent_parser):
|
||||
# FIXME(fabgia): identity related parameters should be passed by the
|
||||
# Keystone client itself to avoid constant update in all the services
|
||||
# clients. When this fix is merged this method can be made obsolete.
|
||||
# Bug: https://bugs.launchpad.net/python-keystoneclient/+bug/1332337
|
||||
parser = parent_parser.add_argument_group('identity')
|
||||
parser.add_argument('-k', '--insecure',
|
||||
parser = parent_parser.add_argument_group("identity")
|
||||
parser.add_argument("-k", "--insecure",
|
||||
default=False,
|
||||
action='store_true',
|
||||
action="store_true",
|
||||
help="Explicitly allow osprofiler to "
|
||||
"perform \"insecure\" SSL (https) requests. "
|
||||
"The server's certificate will "
|
||||
|
|
@ -105,113 +105,113 @@ class OSProfilerShell(object):
|
|||
"caution.")
|
||||
|
||||
# User related options
|
||||
parser.add_argument('--os-username',
|
||||
default=cliutils.env('OS_USERNAME'),
|
||||
help='Defaults to env[OS_USERNAME].')
|
||||
parser.add_argument("--os-username",
|
||||
default=cliutils.env("OS_USERNAME"),
|
||||
help="Defaults to env[OS_USERNAME].")
|
||||
|
||||
parser.add_argument('--os-user-id',
|
||||
default=cliutils.env('OS_USER_ID'),
|
||||
help='Defaults to env[OS_USER_ID].')
|
||||
parser.add_argument("--os-user-id",
|
||||
default=cliutils.env("OS_USER_ID"),
|
||||
help="Defaults to env[OS_USER_ID].")
|
||||
|
||||
parser.add_argument('--os-password',
|
||||
default=cliutils.env('OS_PASSWORD'),
|
||||
help='Defaults to env[OS_PASSWORD].')
|
||||
parser.add_argument("--os-password",
|
||||
default=cliutils.env("OS_PASSWORD"),
|
||||
help="Defaults to env[OS_PASSWORD].")
|
||||
|
||||
# Domain related options
|
||||
parser.add_argument('--os-user-domain-id',
|
||||
default=cliutils.env('OS_USER_DOMAIN_ID'),
|
||||
help='Defaults to env[OS_USER_DOMAIN_ID].')
|
||||
parser.add_argument("--os-user-domain-id",
|
||||
default=cliutils.env("OS_USER_DOMAIN_ID"),
|
||||
help="Defaults to env[OS_USER_DOMAIN_ID].")
|
||||
|
||||
parser.add_argument('--os-user-domain-name',
|
||||
default=cliutils.env('OS_USER_DOMAIN_NAME'),
|
||||
help='Defaults to env[OS_USER_DOMAIN_NAME].')
|
||||
parser.add_argument("--os-user-domain-name",
|
||||
default=cliutils.env("OS_USER_DOMAIN_NAME"),
|
||||
help="Defaults to env[OS_USER_DOMAIN_NAME].")
|
||||
|
||||
parser.add_argument('--os-project-domain-id',
|
||||
default=cliutils.env('OS_PROJECT_DOMAIN_ID'),
|
||||
help='Defaults to env[OS_PROJECT_DOMAIN_ID].')
|
||||
parser.add_argument("--os-project-domain-id",
|
||||
default=cliutils.env("OS_PROJECT_DOMAIN_ID"),
|
||||
help="Defaults to env[OS_PROJECT_DOMAIN_ID].")
|
||||
|
||||
parser.add_argument('--os-project-domain-name',
|
||||
default=cliutils.env('OS_PROJECT_DOMAIN_NAME'),
|
||||
help='Defaults to env[OS_PROJECT_DOMAIN_NAME].')
|
||||
parser.add_argument("--os-project-domain-name",
|
||||
default=cliutils.env("OS_PROJECT_DOMAIN_NAME"),
|
||||
help="Defaults to env[OS_PROJECT_DOMAIN_NAME].")
|
||||
|
||||
# Project V3 or Tenant V2 related options
|
||||
parser.add_argument('--os-project-id',
|
||||
default=cliutils.env('OS_PROJECT_ID'),
|
||||
help='Another way to specify tenant ID. '
|
||||
'This option is mutually exclusive with '
|
||||
' --os-tenant-id. '
|
||||
'Defaults to env[OS_PROJECT_ID].')
|
||||
parser.add_argument("--os-project-id",
|
||||
default=cliutils.env("OS_PROJECT_ID"),
|
||||
help="Another way to specify tenant ID. "
|
||||
"This option is mutually exclusive with "
|
||||
" --os-tenant-id. "
|
||||
"Defaults to env[OS_PROJECT_ID].")
|
||||
|
||||
parser.add_argument('--os-project-name',
|
||||
default=cliutils.env('OS_PROJECT_NAME'),
|
||||
help='Another way to specify tenant name. '
|
||||
'This option is mutually exclusive with '
|
||||
' --os-tenant-name. '
|
||||
'Defaults to env[OS_PROJECT_NAME].')
|
||||
parser.add_argument("--os-project-name",
|
||||
default=cliutils.env("OS_PROJECT_NAME"),
|
||||
help="Another way to specify tenant name. "
|
||||
"This option is mutually exclusive with "
|
||||
" --os-tenant-name. "
|
||||
"Defaults to env[OS_PROJECT_NAME].")
|
||||
|
||||
parser.add_argument('--os-tenant-id',
|
||||
default=cliutils.env('OS_TENANT_ID'),
|
||||
help='This option is mutually exclusive with '
|
||||
' --os-project-id. '
|
||||
'Defaults to env[OS_PROJECT_ID].')
|
||||
parser.add_argument("--os-tenant-id",
|
||||
default=cliutils.env("OS_TENANT_ID"),
|
||||
help="This option is mutually exclusive with "
|
||||
" --os-project-id. "
|
||||
"Defaults to env[OS_PROJECT_ID].")
|
||||
|
||||
parser.add_argument('--os-tenant-name',
|
||||
default=cliutils.env('OS_TENANT_NAME'),
|
||||
help='Defaults to env[OS_TENANT_NAME].')
|
||||
parser.add_argument("--os-tenant-name",
|
||||
default=cliutils.env("OS_TENANT_NAME"),
|
||||
help="Defaults to env[OS_TENANT_NAME].")
|
||||
|
||||
# Auth related options
|
||||
parser.add_argument('--os-auth-url',
|
||||
default=cliutils.env('OS_AUTH_URL'),
|
||||
help='Defaults to env[OS_AUTH_URL].')
|
||||
parser.add_argument("--os-auth-url",
|
||||
default=cliutils.env("OS_AUTH_URL"),
|
||||
help="Defaults to env[OS_AUTH_URL].")
|
||||
|
||||
parser.add_argument('--os-auth-token',
|
||||
default=cliutils.env('OS_AUTH_TOKEN'),
|
||||
help='Defaults to env[OS_AUTH_TOKEN].')
|
||||
parser.add_argument("--os-auth-token",
|
||||
default=cliutils.env("OS_AUTH_TOKEN"),
|
||||
help="Defaults to env[OS_AUTH_TOKEN].")
|
||||
|
||||
parser.add_argument('--os-cacert',
|
||||
metavar='<ca-certificate-file>',
|
||||
dest='os_cacert',
|
||||
default=cliutils.env('OS_CACERT'),
|
||||
help='Path of CA TLS certificate(s) used to verify'
|
||||
' the remote server\'s certificate. Without this '
|
||||
'option ceilometer looks for the default system CA'
|
||||
' certificates.')
|
||||
parser.add_argument("--os-cacert",
|
||||
metavar="<ca-certificate-file>",
|
||||
dest="os_cacert",
|
||||
default=cliutils.env("OS_CACERT"),
|
||||
help="Path of CA TLS certificate(s) used to verify"
|
||||
" the remote server\"s certificate. Without this "
|
||||
"option ceilometer looks for the default system CA"
|
||||
" certificates.")
|
||||
|
||||
parser.add_argument('--os-cert',
|
||||
help='Path of certificate file to use in SSL '
|
||||
'connection. This file can optionally be '
|
||||
'prepended with the private key.')
|
||||
parser.add_argument("--os-cert",
|
||||
help="Path of certificate file to use in SSL "
|
||||
"connection. This file can optionally be "
|
||||
"prepended with the private key.")
|
||||
|
||||
parser.add_argument('--os-key',
|
||||
help='Path of client key to use in SSL '
|
||||
'connection. This option is not necessary '
|
||||
'if your key is prepended to your cert file.')
|
||||
parser.add_argument("--os-key",
|
||||
help="Path of client key to use in SSL "
|
||||
"connection. This option is not necessary "
|
||||
"if your key is prepended to your cert file.")
|
||||
|
||||
# Service Catalog related options
|
||||
parser.add_argument('--os-service-type',
|
||||
default=cliutils.env('OS_SERVICE_TYPE'),
|
||||
help='Defaults to env[OS_SERVICE_TYPE].')
|
||||
parser.add_argument("--os-service-type",
|
||||
default=cliutils.env("OS_SERVICE_TYPE"),
|
||||
help="Defaults to env[OS_SERVICE_TYPE].")
|
||||
|
||||
parser.add_argument('--os-endpoint-type',
|
||||
default=cliutils.env('OS_ENDPOINT_TYPE'),
|
||||
help='Defaults to env[OS_ENDPOINT_TYPE].')
|
||||
parser.add_argument("--os-endpoint-type",
|
||||
default=cliutils.env("OS_ENDPOINT_TYPE"),
|
||||
help="Defaults to env[OS_ENDPOINT_TYPE].")
|
||||
|
||||
parser.add_argument('--os-region-name',
|
||||
default=cliutils.env('OS_REGION_NAME'),
|
||||
help='Defaults to env[OS_REGION_NAME].')
|
||||
parser.add_argument("--os-region-name",
|
||||
default=cliutils.env("OS_REGION_NAME"),
|
||||
help="Defaults to env[OS_REGION_NAME].")
|
||||
|
||||
def _append_subcommands(self, parent_parser):
|
||||
subcommands = parent_parser.add_subparsers(help='<subcommands>')
|
||||
subcommands = parent_parser.add_subparsers(help="<subcommands>")
|
||||
for group_cls in commands.BaseCommand.__subclasses__():
|
||||
group_parser = subcommands.add_parser(group_cls.group_name)
|
||||
subcommand_parser = group_parser.add_subparsers()
|
||||
|
||||
for name, callback in inspect.getmembers(
|
||||
group_cls(), predicate=inspect.ismethod):
|
||||
command = name.replace('_', '-')
|
||||
desc = callback.__doc__ or ''
|
||||
help_message = desc.strip().split('\n')[0]
|
||||
arguments = getattr(callback, 'arguments', [])
|
||||
command = name.replace("_", "-")
|
||||
desc = callback.__doc__ or ""
|
||||
help_message = desc.strip().split("\n")[0]
|
||||
arguments = getattr(callback, "arguments", [])
|
||||
|
||||
command_parser = subcommand_parser.add_parser(
|
||||
command, help=help_message, description=desc)
|
||||
|
|
|
|||
|
|
@ -55,6 +55,6 @@ def create(plugin_name, *args, **kwargs):
|
|||
:param *args: args that will be passed to plugin init method
|
||||
:param **kwargs: kwargs that will be passed to plugin init method
|
||||
:returns: Callable notifier method
|
||||
:raise TypeError: In case of invalid name of plugin raises TypeError
|
||||
:raises TypeError: In case of invalid name of plugin raises TypeError
|
||||
"""
|
||||
return base.Notifier.factory(plugin_name, *args, **kwargs)
|
||||
|
|
|
|||
|
|
@ -126,6 +126,6 @@ def get_notifications(ceilometer, base_id):
|
|||
:param base_id: Base id of trace elements.
|
||||
"""
|
||||
|
||||
_filter = '{"=": {"resource_id": "profiler-%s"}}' % base_id
|
||||
_filter = "{\"=\": {\"resource_id\": \"profiler-%s\"}}" % base_id
|
||||
return [n.to_dict()
|
||||
for n in ceilometer.query_samples.query(_filter, None, None)]
|
||||
|
|
|
|||
|
|
@ -243,7 +243,7 @@ class _Profiler(object):
|
|||
|
||||
:param info: Dict with useful info. It will be send in notification.
|
||||
"""
|
||||
self._notify('%s-stop' % self._name.pop(), info)
|
||||
self._notify("%s-stop" % self._name.pop(), info)
|
||||
self._trace_stack.pop()
|
||||
|
||||
def _notify(self, name, info):
|
||||
|
|
|
|||
|
|
@ -36,9 +36,9 @@ def add_tracing(sqlalchemy, engine, name):
|
|||
"""Add tracing to all sqlalchemy calls."""
|
||||
|
||||
if not _DISABLED:
|
||||
sqlalchemy.event.listen(engine, 'before_cursor_execute',
|
||||
sqlalchemy.event.listen(engine, "before_cursor_execute",
|
||||
_before_cursor_execute(name))
|
||||
sqlalchemy.event.listen(engine, 'after_cursor_execute',
|
||||
sqlalchemy.event.listen(engine, "after_cursor_execute",
|
||||
_after_cursor_execute())
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,378 @@
|
|||
# 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 OSProfiler specific tests. OpenStack general tests
|
||||
should be submitted to the common 'hacking' module.
|
||||
- Pick numbers in the range N3xx. 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 N3xx value.
|
||||
- List the new rule in the top level HACKING.rst file
|
||||
- Add test cases for each new rule to tests/unit/test_hacking.py
|
||||
|
||||
"""
|
||||
|
||||
import functools
|
||||
import re
|
||||
import tokenize
|
||||
|
||||
re_assert_true_instance = re.compile(
|
||||
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
|
||||
r"(\w|\.|\'|\"|\[|\])+\)\)")
|
||||
re_assert_equal_type = re.compile(
|
||||
r"(.)*assertEqual\(type\((\w|\.|\'|\"|\[|\])+\), "
|
||||
r"(\w|\.|\'|\"|\[|\])+\)")
|
||||
re_assert_equal_end_with_none = re.compile(r"assertEqual\(.*?,\s+None\)$")
|
||||
re_assert_equal_start_with_none = re.compile(r"assertEqual\(None,")
|
||||
re_assert_true_false_with_in_or_not_in = re.compile(
|
||||
r"assert(True|False)\("
|
||||
r"(\w|[][.'\"])+( not)? in (\w|[][.'\",])+(, .*)?\)")
|
||||
re_assert_true_false_with_in_or_not_in_spaces = re.compile(
|
||||
r"assert(True|False)\((\w|[][.'\"])+( not)? in [\[|'|\"](\w|[][.'\", ])+"
|
||||
r"[\[|'|\"](, .*)?\)")
|
||||
re_assert_equal_in_end_with_true_or_false = re.compile(
|
||||
r"assertEqual\((\w|[][.'\"])+( not)? in (\w|[][.'\", ])+, (True|False)\)")
|
||||
re_assert_equal_in_start_with_true_or_false = re.compile(
|
||||
r"assertEqual\((True|False), (\w|[][.'\"])+( not)? in (\w|[][.'\", ])+\)")
|
||||
re_no_construct_dict = re.compile(
|
||||
r"\sdict\(\)")
|
||||
re_no_construct_list = re.compile(
|
||||
r"\slist\(\)")
|
||||
re_str_format = re.compile(r"""
|
||||
% # start of specifier
|
||||
\(([^)]+)\) # mapping key, in group 1
|
||||
[#0 +\-]? # optional conversion flag
|
||||
(?:-?\d*)? # optional minimum field width
|
||||
(?:\.\d*)? # optional precision
|
||||
[hLl]? # optional length modifier
|
||||
[A-z%] # conversion modifier
|
||||
""", re.X)
|
||||
re_raises = re.compile(
|
||||
r"\s:raise[^s] *.*$|\s:raises *:.*$|\s:raises *[^:]+$")
|
||||
|
||||
|
||||
def skip_ignored_lines(func):
|
||||
|
||||
@functools.wraps(func)
|
||||
def wrapper(logical_line, filename):
|
||||
line = logical_line.strip()
|
||||
if not line or line.startswith("#") or line.endswith("# noqa"):
|
||||
return
|
||||
yield next(func(logical_line, filename))
|
||||
|
||||
return wrapper
|
||||
|
||||
|
||||
def _parse_assert_mock_str(line):
|
||||
point = line.find(".assert_")
|
||||
|
||||
if point != -1:
|
||||
end_pos = line[point:].find("(") + point
|
||||
return point, line[point + 1: end_pos], line[: point]
|
||||
else:
|
||||
return None, None, None
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def check_assert_methods_from_mock(logical_line, filename):
|
||||
"""Ensure that ``assert_*`` methods from ``mock`` library is used correctly
|
||||
|
||||
N301 - base error number
|
||||
N302 - related to nonexistent "assert_called"
|
||||
N303 - related to nonexistent "assert_called_once"
|
||||
"""
|
||||
|
||||
correct_names = ["assert_any_call", "assert_called_once_with",
|
||||
"assert_called_with", "assert_has_calls"]
|
||||
ignored_files = ["./tests/unit/test_hacking.py"]
|
||||
|
||||
if filename.startswith("./tests") and filename not in ignored_files:
|
||||
pos, method_name, obj_name = _parse_assert_mock_str(logical_line)
|
||||
|
||||
if pos:
|
||||
if method_name not in correct_names:
|
||||
error_number = "N301"
|
||||
msg = ("%(error_number)s:'%(method)s' is not present in `mock`"
|
||||
" library. %(custom_msg)s For more details, visit "
|
||||
"http://www.voidspace.org.uk/python/mock/ .")
|
||||
|
||||
if method_name == "assert_called":
|
||||
error_number = "N302"
|
||||
custom_msg = ("Maybe, you should try to use "
|
||||
"'assertTrue(%s.called)' instead." %
|
||||
obj_name)
|
||||
elif method_name == "assert_called_once":
|
||||
# For more details, see a bug in Rally:
|
||||
# https://bugs.launchpad.net/rally/+bug/1305991
|
||||
error_number = "N303"
|
||||
custom_msg = ("Maybe, you should try to use "
|
||||
"'assertEqual(1, %s.call_count)' "
|
||||
"or '%s.assert_called_once_with()'"
|
||||
" instead." % (obj_name, obj_name))
|
||||
else:
|
||||
custom_msg = ("Correct 'assert_*' methods: '%s'."
|
||||
% "', '".join(correct_names))
|
||||
|
||||
yield (pos, msg % {
|
||||
"error_number": error_number,
|
||||
"method": method_name,
|
||||
"custom_msg": custom_msg})
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def assert_true_instance(logical_line, filename):
|
||||
"""Check for assertTrue(isinstance(a, b)) sentences
|
||||
|
||||
N320
|
||||
"""
|
||||
if re_assert_true_instance.match(logical_line):
|
||||
yield (0, "N320 assertTrue(isinstance(a, b)) sentences not allowed, "
|
||||
"you should use assertIsInstance(a, b) instead.")
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def assert_equal_type(logical_line, filename):
|
||||
"""Check for assertEqual(type(A), B) sentences
|
||||
|
||||
N321
|
||||
"""
|
||||
if re_assert_equal_type.match(logical_line):
|
||||
yield (0, "N321 assertEqual(type(A), B) sentences not allowed, "
|
||||
"you should use assertIsInstance(a, b) instead.")
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def assert_equal_none(logical_line, filename):
|
||||
"""Check for assertEqual(A, None) or assertEqual(None, A) sentences
|
||||
|
||||
N322
|
||||
"""
|
||||
res = (re_assert_equal_start_with_none.search(logical_line) or
|
||||
re_assert_equal_end_with_none.search(logical_line))
|
||||
if res:
|
||||
yield (0, "N322 assertEqual(A, None) or assertEqual(None, A) "
|
||||
"sentences not allowed, you should use assertIsNone(A) "
|
||||
"instead.")
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def assert_true_or_false_with_in(logical_line, filename):
|
||||
"""Check assertTrue/False(A in/not in B) with collection contents
|
||||
|
||||
Check for assertTrue/False(A in B), assertTrue/False(A not in B),
|
||||
assertTrue/False(A in B, message) or assertTrue/False(A not in B, message)
|
||||
sentences.
|
||||
|
||||
N323
|
||||
"""
|
||||
res = (re_assert_true_false_with_in_or_not_in.search(logical_line) or
|
||||
re_assert_true_false_with_in_or_not_in_spaces.search(logical_line))
|
||||
if res:
|
||||
yield (0, "N323 assertTrue/assertFalse(A in/not in B)sentences not "
|
||||
"allowed, you should use assertIn(A, B) or assertNotIn(A, B)"
|
||||
" instead.")
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def assert_equal_in(logical_line, filename):
|
||||
"""Check assertEqual(A in/not in B, True/False) with collection contents
|
||||
|
||||
Check for assertEqual(A in B, True/False), assertEqual(True/False, A in B),
|
||||
assertEqual(A not in B, True/False) or assertEqual(True/False, A not in B)
|
||||
sentences.
|
||||
|
||||
N324
|
||||
"""
|
||||
res = (re_assert_equal_in_end_with_true_or_false.search(logical_line) or
|
||||
re_assert_equal_in_start_with_true_or_false.search(logical_line))
|
||||
if res:
|
||||
yield (0, "N324: Use assertIn/NotIn(A, B) rather than "
|
||||
"assertEqual(A in/not in B, True/False) when checking "
|
||||
"collection contents.")
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def check_quotes(logical_line, filename):
|
||||
"""Check that single quotation marks are not used
|
||||
|
||||
N350
|
||||
"""
|
||||
|
||||
in_string = False
|
||||
in_multiline_string = False
|
||||
single_quotas_are_used = False
|
||||
|
||||
check_tripple = (
|
||||
lambda line, i, char: (
|
||||
i + 2 < len(line) and
|
||||
(char == line[i] == line[i + 1] == line[i + 2])
|
||||
)
|
||||
)
|
||||
|
||||
i = 0
|
||||
while i < len(logical_line):
|
||||
char = logical_line[i]
|
||||
|
||||
if in_string:
|
||||
if char == "\"":
|
||||
in_string = False
|
||||
if char == "\\":
|
||||
i += 1 # ignore next char
|
||||
|
||||
elif in_multiline_string:
|
||||
if check_tripple(logical_line, i, "\""):
|
||||
i += 2 # skip next 2 chars
|
||||
in_multiline_string = False
|
||||
|
||||
elif char == "#":
|
||||
break
|
||||
|
||||
elif char == "'":
|
||||
single_quotas_are_used = True
|
||||
break
|
||||
|
||||
elif char == "\"":
|
||||
if check_tripple(logical_line, i, "\""):
|
||||
in_multiline_string = True
|
||||
i += 3
|
||||
continue
|
||||
in_string = True
|
||||
|
||||
i += 1
|
||||
|
||||
if single_quotas_are_used:
|
||||
yield (i, "N350 Remove Single quotes")
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def check_no_constructor_data_struct(logical_line, filename):
|
||||
"""Check that data structs (lists, dicts) are declared using literals
|
||||
|
||||
N351
|
||||
"""
|
||||
|
||||
match = re_no_construct_dict.search(logical_line)
|
||||
if match:
|
||||
yield (0, "N351 Remove dict() construct and use literal {}")
|
||||
match = re_no_construct_list.search(logical_line)
|
||||
if match:
|
||||
yield (0, "N351 Remove list() construct and use literal []")
|
||||
|
||||
|
||||
def check_dict_formatting_in_string(logical_line, tokens):
|
||||
"""Check that strings do not use dict-formatting with a single replacement
|
||||
|
||||
N352
|
||||
"""
|
||||
# NOTE(stpierre): Can't use @skip_ignored_lines here because it's
|
||||
# a stupid decorator that only works on functions that take
|
||||
# (logical_line, filename) as arguments.
|
||||
if (not logical_line or
|
||||
logical_line.startswith("#") or
|
||||
logical_line.endswith("# noqa")):
|
||||
return
|
||||
|
||||
current_string = ""
|
||||
in_string = False
|
||||
for token_type, text, start, end, line in tokens:
|
||||
if token_type == tokenize.STRING:
|
||||
if not in_string:
|
||||
current_string = ""
|
||||
in_string = True
|
||||
current_string += text.strip("\"")
|
||||
elif token_type == tokenize.OP:
|
||||
if not current_string:
|
||||
continue
|
||||
# NOTE(stpierre): The string formatting operator % has
|
||||
# lower precedence than +, so we assume that the logical
|
||||
# string has concluded whenever we hit an operator of any
|
||||
# sort. (Most operators don't work for strings anyway.)
|
||||
# Some string operators do have higher precedence than %,
|
||||
# though, so you can technically trick this check by doing
|
||||
# things like:
|
||||
#
|
||||
# "%(foo)s" * 1 % {"foo": 1}
|
||||
# "%(foo)s"[:] % {"foo": 1}
|
||||
#
|
||||
# It also will produce false positives if you use explicit
|
||||
# parenthesized addition for two strings instead of
|
||||
# concatenation by juxtaposition, e.g.:
|
||||
#
|
||||
# ("%(foo)s" + "%(bar)s") % vals
|
||||
#
|
||||
# But if you do any of those things, then you deserve all
|
||||
# of the horrible things that happen to you, and probably
|
||||
# many more.
|
||||
in_string = False
|
||||
if text == "%":
|
||||
format_keys = set()
|
||||
for match in re_str_format.finditer(current_string):
|
||||
format_keys.add(match.group(1))
|
||||
if len(format_keys) == 1:
|
||||
yield (0,
|
||||
"N353 Do not use mapping key string formatting "
|
||||
"with a single key")
|
||||
if text != ")":
|
||||
# NOTE(stpierre): You can have a parenthesized string
|
||||
# followed by %, so a closing paren doesn't obviate
|
||||
# the possibility for a substitution operator like
|
||||
# every other operator does.
|
||||
current_string = ""
|
||||
elif token_type in (tokenize.NL, tokenize.COMMENT):
|
||||
continue
|
||||
else:
|
||||
in_string = False
|
||||
if token_type == tokenize.NEWLINE:
|
||||
current_string = ""
|
||||
|
||||
|
||||
@skip_ignored_lines
|
||||
def check_using_unicode(logical_line, filename):
|
||||
"""Check crosspython unicode usage
|
||||
|
||||
N353
|
||||
"""
|
||||
|
||||
if re.search(r"\bunicode\(", logical_line):
|
||||
yield (0, "N353 'unicode' function is absent in python3. Please "
|
||||
"use 'six.text_type' instead.")
|
||||
|
||||
|
||||
def check_raises(physical_line, filename):
|
||||
"""Check raises usage
|
||||
|
||||
N354
|
||||
"""
|
||||
|
||||
ignored_files = ["./tests/unit/test_hacking.py",
|
||||
"./tests/hacking/checks.py"]
|
||||
if filename not in ignored_files:
|
||||
if re_raises.search(physical_line):
|
||||
return (0, "N354 ':Please use ':raises Exception: conditions' "
|
||||
"in docstrings.")
|
||||
|
||||
|
||||
def factory(register):
|
||||
register(check_assert_methods_from_mock)
|
||||
register(assert_true_instance)
|
||||
register(assert_equal_type)
|
||||
register(assert_equal_none)
|
||||
register(assert_true_or_false_with_in)
|
||||
register(assert_equal_in)
|
||||
register(check_quotes)
|
||||
register(check_no_constructor_data_struct)
|
||||
register(check_dict_formatting_in_string)
|
||||
register(check_using_unicode)
|
||||
register(check_raises)
|
||||
|
|
@ -247,7 +247,8 @@ class CeilometerParserTestCase(test.TestCase):
|
|||
|
||||
result = ceilometer.get_notifications(mock_ceil_client, base_id)
|
||||
|
||||
expected_filter = '{"=": {"resource_id": "profiler-%s"}}' % base_id
|
||||
expected_filter = (
|
||||
"{\"=\": {\"resource_id\": \"profiler-%s\"}}" % base_id)
|
||||
mock_ceil_client.query_samples.query.assert_called_once_with(
|
||||
expected_filter, None, None)
|
||||
self.assertEqual(result, [results[0].to_dict(), results[1].to_dict()])
|
||||
|
|
|
|||
|
|
@ -67,21 +67,21 @@ class UtilsTestCase(test.TestCase):
|
|||
|
||||
process_data = utils.signed_unpack(packed_data, hmac_data, [hmac])
|
||||
self.assertIn("hmac_key", process_data)
|
||||
process_data.pop('hmac_key')
|
||||
process_data.pop("hmac_key")
|
||||
self.assertEqual(data, process_data)
|
||||
|
||||
def test_signed_pack_unpack_many_keys(self):
|
||||
keys = ['secret', 'secret2', 'secret3']
|
||||
keys = ["secret", "secret2", "secret3"]
|
||||
data = {"some": "data"}
|
||||
packed_data, hmac_data = utils.signed_pack(data, keys[-1])
|
||||
|
||||
process_data = utils.signed_unpack(packed_data, hmac_data, keys)
|
||||
self.assertEqual(keys[-1], process_data['hmac_key'])
|
||||
self.assertEqual(keys[-1], process_data["hmac_key"])
|
||||
|
||||
def test_signed_pack_unpack_many_wrong_keys(self):
|
||||
keys = ['secret', 'secret2', 'secret3']
|
||||
keys = ["secret", "secret2", "secret3"]
|
||||
data = {"some": "data"}
|
||||
packed_data, hmac_data = utils.signed_pack(data, 'password')
|
||||
packed_data, hmac_data = utils.signed_pack(data, "password")
|
||||
|
||||
process_data = utils.signed_unpack(packed_data, hmac_data, keys)
|
||||
self.assertIsNone(process_data)
|
||||
|
|
|
|||
|
|
@ -48,9 +48,9 @@ class WebTestCase(test.TestCase):
|
|||
|
||||
trace_info = utils.signed_unpack(headers["X-Trace-Info"],
|
||||
headers["X-Trace-HMAC"], ["key"])
|
||||
self.assertIn('hmac_key', trace_info)
|
||||
self.assertEqual('key', trace_info.pop('hmac_key'))
|
||||
self.assertEqual({"parent_id": 'z', 'base_id': 'y'}, trace_info)
|
||||
self.assertIn("hmac_key", trace_info)
|
||||
self.assertEqual("key", trace_info.pop("hmac_key"))
|
||||
self.assertEqual({"parent_id": "z", "base_id": "y"}, trace_info)
|
||||
|
||||
@mock.patch("osprofiler.profiler.get")
|
||||
def test_get_trace_id_headers_no_profiler(self, mock_get_profiler):
|
||||
|
|
@ -280,7 +280,7 @@ class WebMiddlewareTestCase(test.TestCase):
|
|||
request.query_string = "query"
|
||||
request.method = "method"
|
||||
request.scheme = "scheme"
|
||||
hmac_key = 'super_secret_key2'
|
||||
hmac_key = "super_secret_key2"
|
||||
|
||||
pack = utils.signed_pack({"base_id": "1", "parent_id": "2"}, hmac_key)
|
||||
request.headers = {
|
||||
|
|
@ -290,7 +290,7 @@ class WebMiddlewareTestCase(test.TestCase):
|
|||
"X-Trace-HMAC": pack[1]
|
||||
}
|
||||
|
||||
web.enable('super_secret_key1,super_secret_key2')
|
||||
web.enable("super_secret_key1,super_secret_key2")
|
||||
middleware = web.WsgiMiddleware("app", enabled=True)
|
||||
self.assertEqual("yeah!", middleware(request))
|
||||
mock_profiler_init.assert_called_once_with(hmac_key=hmac_key,
|
||||
|
|
|
|||
Loading…
Reference in New Issue