From 7ce9ec92b4b01910c0e2ee264ba4a72c4f8642fe Mon Sep 17 00:00:00 2001 From: Boris Pavlovic Date: Sat, 14 Mar 2015 04:39:59 +0300 Subject: [PATCH] Improve hacking and remove useless unit tests * Remove tests/unit/aas and test/unit/fixture fixtures are not used so just remove them aas - didn't have __init__.py files so these tests were not executed at all. By the way code related to this tests was removed earlier in this patch: https://review.openstack.org/#/c/152847/ * Remove all old py3 related hacking rules Now we have unit & functional jobs that checks them * Add new hacking rule that protect us from usage of ' * Unified signature of all hacking methods (otherwise we are not able to make decorator) * Add hacking rules decorator for skiping 3 types of lines: empty, commented, and with # noqa comment Change-Id: I6bf2cc123325a4980edeb066a6c31aa685858f5b --- tests/hacking/README.rst | 13 +- tests/hacking/checks.py | 237 +++++------- tests/unit/aas/rest/base.py | 223 ----------- tests/unit/aas/rest/controllers/test_root.py | 44 --- tests/unit/common/test_utils.py | 12 - tests/unit/fixtures/__init__.py | 0 tests/unit/fixtures/import/__init__.py | 0 tests/unit/fixtures/import/broken.py | 5 - .../unit/fixtures/import/package/__init__.py | 0 tests/unit/fixtures/import/package/a.py | 6 - tests/unit/fixtures/import/package/b.py | 7 - tests/unit/test_hacking.py | 356 ++++++------------ 12 files changed, 204 insertions(+), 699 deletions(-) delete mode 100644 tests/unit/aas/rest/base.py delete mode 100644 tests/unit/aas/rest/controllers/test_root.py delete mode 100644 tests/unit/fixtures/__init__.py delete mode 100644 tests/unit/fixtures/import/__init__.py delete mode 100644 tests/unit/fixtures/import/broken.py delete mode 100644 tests/unit/fixtures/import/package/__init__.py delete mode 100644 tests/unit/fixtures/import/package/a.py delete mode 100644 tests/unit/fixtures/import/package/b.py diff --git a/tests/hacking/README.rst b/tests/hacking/README.rst index 1102b8b9e8..7cc7b264ef 100644 --- a/tests/hacking/README.rst +++ b/tests/hacking/README.rst @@ -21,15 +21,6 @@ Rally Specific Commandments * [N322] - Ensure that ``assertEqual(A, None)`` and ``assertEqual(None, A)`` are not used * [N323] - Ensure that ``assertTrue/assertFalse(A in/not in B)`` are not used with collection contents * [N324] - Ensure that ``assertEqual(A in/not in B, True/False)`` and ``assertEqual(True/False, A in/not in B)`` are not used with collection contents -* [N33x] - Reserved for rules related to Python 3 compatibility - * [N330] - Ensure that ``dict.iterkeys()``, ``dict.itervalues()``, ``dict.iteritems()`` and ``dict.iterlist()`` are not used - * [N331] - Ensure that ``basestring`` is not used - * [N332] - Ensure that ``StringIO.StringIO`` is not used - * [N333] - Ensure that ``urlparse`` is not used - * [N334] - Ensure that ``itertools.imap`` is not used - * [N335] - Ensure that ``xrange`` is not used - * [N336] - Ensure that ``string.lowercase`` and ``string.uppercase`` are not used - * [N337] - Ensure that ``next()`` method on iterator objects is not used - * [N338] - Ensure that ``+`` operand is not used to concatenate dict.items() * [N340] - Ensure that we are importing always ``from rally import objects`` -* [N341] - Ensure that we are importing oslo_xyz packages instead of deprecated oslo.xyz ones \ No newline at end of file +* [N341] - Ensure that we are importing oslo_xyz packages instead of deprecated oslo.xyz ones +* [N350] - Ensure that single quotes are not used diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py index 252cc44879..4a3d5c292f 100644 --- a/tests/hacking/checks.py +++ b/tests/hacking/checks.py @@ -24,6 +24,7 @@ Guidelines for writing new hacking checks """ +import functools import re @@ -45,16 +46,18 @@ 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_basestring_method = re.compile(r"(^|[\s,(\[=])basestring([\s,)\]]|$)") -re_StringIO_method = re.compile(r"StringIO\.StringIO\(") -re_urlparse_method = re.compile(r"(^|[\s=])urlparse\.") -re_itertools_imap_method = re.compile(r"(^|[\s=])itertools\.imap\(") -re_xrange_method = re.compile(r"(^|[\s=])xrange\(") -re_string_lower_upper_case_method = re.compile( - r"(^|[(,\s=])string\.(lower|upper)case([)\[,\s]|$)") -re_next_on_iterator_method = re.compile(r"\.next\(\)") -re_concatenate_dict = re.compile( - r".*\.items\(\)(\s*\+\s*.*\.items\(\))+.*") + + +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): @@ -67,6 +70,7 @@ def _parse_assert_mock_str(line): 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 @@ -112,6 +116,7 @@ def check_assert_methods_from_mock(logical_line, filename): "custom_msg": custom_msg}) +@skip_ignored_lines def check_import_of_logging(logical_line, filename): """Check correctness import of logging module @@ -131,7 +136,8 @@ def check_import_of_logging(logical_line, filename): "use `rally.common.log` instead.") -def no_translate_debug_logs(logical_line): +@skip_ignored_lines +def no_translate_debug_logs(logical_line, filename): """Check for "LOG.debug(_(" As per our translation policy, @@ -148,6 +154,7 @@ def no_translate_debug_logs(logical_line): yield(0, "N311 Don't translate debug level logs") +@skip_ignored_lines def no_use_conf_debug_check(logical_line, filename): """Check for "cfg.CONF.debug" @@ -167,7 +174,8 @@ def no_use_conf_debug_check(logical_line, filename): "should be used instead.") -def assert_true_instance(logical_line): +@skip_ignored_lines +def assert_true_instance(logical_line, filename): """Check for assertTrue(isinstance(a, b)) sentences N320 @@ -177,7 +185,8 @@ def assert_true_instance(logical_line): "you should use assertIsInstance(a, b) instead.") -def assert_equal_type(logical_line): +@skip_ignored_lines +def assert_equal_type(logical_line, filename): """Check for assertEqual(type(A), B) sentences N321 @@ -187,7 +196,8 @@ def assert_equal_type(logical_line): "you should use assertIsInstance(a, b) instead.") -def assert_equal_none(logical_line): +@skip_ignored_lines +def assert_equal_none(logical_line, filename): """Check for assertEqual(A, None) or assertEqual(None, A) sentences N322 @@ -200,7 +210,8 @@ def assert_equal_none(logical_line): "instead.") -def assert_true_or_false_with_in(logical_line): +@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), @@ -217,7 +228,8 @@ def assert_true_or_false_with_in(logical_line): " instead.") -def assert_equal_in(logical_line): +@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), @@ -234,134 +246,7 @@ def assert_equal_in(logical_line): "collection contents.") -def check_iteritems_method(logical_line): - """Check that collections are iterated in Python 3 compatible way - - The correct forms: - six.iterkeys(collection) - six.itervalues(collection) - six.iteritems(collection) - six.iterlist(collection) - - N330 - """ - iter_functions = ["iterkeys()", "itervalues()", - "iteritems()", "iterlist()"] - for func in iter_functions: - pos = logical_line.find(func) - if pos != -1: - yield (pos, "N330: Use six.%(func)s(dict) rather than " - "dict.%(func)s() to iterate a collection." % - {"func": func[:-2]}) - - -def check_basestring_method(logical_line): - """Check if basestring is properly called for compatibility with Python 3 - - There is no global variable "basestring" in Python 3.The correct form - is six.string_types, instead of basestring. - - N331 - """ - res = re_basestring_method.search(logical_line) - if res: - yield (0, "N331: Use six.string_types rather than basestring.") - - -def check_StringIO_method(logical_line): - """Check if StringIO is properly called for compatibility with Python 3 - - In Python 3, StringIO module is gone. The correct form is - six.moves.StringIO instead of StringIO.StringIO. - - N332 - """ - res = re_StringIO_method.search(logical_line) - if res: - yield (0, "N332: Use six.moves.StringIO " - "rather than StringIO.StringIO.") - - -def check_urlparse_method(logical_line): - """Check if urlparse is properly called for compatibility with Python 3 - - The correct form is six.moves.urllib.parse instead of "urlparse". - - N333 - """ - res = re_urlparse_method.search(logical_line) - if res: - yield (0, "N333: Use six.moves.urllib.parse rather than urlparse.") - - -def check_itertools_imap_method(logical_line): - """Check if itertools.imap is properly called for compatibility with Python 3 - - The correct form is six.moves.map instead of itertools.imap. - - N334 - """ - res = re_itertools_imap_method.search(logical_line) - if res: - yield (0, "N334: Use six.moves.map rather than itertools.imap.") - - -def check_xrange_method(logical_line): - """Check if xrange is properly called for compatibility with Python 3 - - The correct form is six.moves.range instead of xrange. - - N335 - """ - res = re_xrange_method.search(logical_line) - if res: - yield (0, "N335: Use six.moves.range rather than xrange.") - - -def check_string_lower_upper_case_method(logical_line): - """Check if string.lowercase and string.uppercase are properly called - - In Python 3, string.lowercase and string.uppercase are gone. - The correct form is "string.ascii_lowercase" and "string.ascii_uppercase". - - N336 - """ - res = re_string_lower_upper_case_method.search(logical_line) - if res: - yield (0, "N336: Use string.ascii_lowercase or string.ascii_uppercase " - "rather than string.lowercase or string.uppercase.") - - -def check_next_on_iterator_method(logical_line): - """Check if next() method on iterator objects are properly called - - Python 3 introduced a next() function to replace the next() method on - iterator objects. Rather than calling the method on the iterator, - the next() function is called with the iterable object as it's sole - parameter, which calls the underlying __next__() method. - - N337 - """ - res = re_next_on_iterator_method.search(logical_line) - if res: - yield (0, "N337: Use next(iterator) rather than iterator.next().") - - -def check_concatenate_dict_with_plus_operand(logical_line): - """Check if a dict is being sum with + operator - - Python 3 dict.items() return a dict_items object instead of a list, and - this object, doesn't support + operator. Need to use the update method - instead in order to concatenate two dictionaries. - - N338 - """ - res = re_concatenate_dict.search(logical_line) - if res: - yield (0, "N338: Use update() method instead of '+'' operand to " - "concatenate dictionaries") - - +@skip_ignored_lines def check_no_direct_rally_objects_import(logical_line, filename): """Check if rally.objects are properly imported. @@ -379,6 +264,7 @@ def check_no_direct_rally_objects_import(logical_line, filename): "After that you can use directly objects e.g. objects.Task") +@skip_ignored_lines def check_no_oslo_deprecated_import(logical_line, filename): """Check if oslo.foo packages are not imported instead of oslo_foo ones. @@ -394,6 +280,59 @@ def check_no_oslo_deprecated_import(logical_line, filename): "instead") +@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") + + def factory(register): register(check_assert_methods_from_mock) register(check_import_of_logging) @@ -404,14 +343,6 @@ def factory(register): register(assert_equal_none) register(assert_true_or_false_with_in) register(assert_equal_in) - register(check_iteritems_method) - register(check_basestring_method) - register(check_StringIO_method) - register(check_urlparse_method) - register(check_itertools_imap_method) - register(check_xrange_method) - register(check_string_lower_upper_case_method) - register(check_next_on_iterator_method) register(check_no_direct_rally_objects_import) - register(check_concatenate_dict_with_plus_operand) register(check_no_oslo_deprecated_import) + register(check_quotes) diff --git a/tests/unit/aas/rest/base.py b/tests/unit/aas/rest/base.py deleted file mode 100644 index 5e9bd25f7c..0000000000 --- a/tests/unit/aas/rest/base.py +++ /dev/null @@ -1,223 +0,0 @@ -# Copyright 2013 Hewlett-Packard Development Company, L.P. -# All Rights Reserved. -# -# 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. -"""Base classes for API tests.""" - -# NOTE: Ported from ceilometer/tests/api.py (subsequently moved to -# ceilometer/tests/api/__init__.py). - -import pecan -import pecan.testing -from six.moves.urllib import parse - -from tests.unit import test - -PATH_PREFIX = "/v1" - - -class PecanControllerTest(test.TestCase): - """Used for functional tests of Pecan controllers. - - Use this class where you need to test your literal application - and its integration with the framework. - """ - - SOURCE_DATA = {"test_source": {"somekey": "666"}} - - def setUp(self): - super(PecanControllerTest, self).setUp() - self.app = self._make_app() - - def reset_pecan(): - pecan.set_config({}, overwrite=True) - - self.addCleanup(reset_pecan) - - def _make_app(self, enable_acl=False): - # Determine where we are so we can set up paths in the config - - self.config = { - "app": { - "root": "rally.aas.rest.controllers.root.RootController", - "modules": ["rally.aas.rest"], - }, - } - - return pecan.testing.load_test_app(self.config) - - def _request_json(self, path, params, expect_errors=False, headers=None, - method="post", extra_environ=None, status=None, - path_prefix=PATH_PREFIX): - """Simulate an json request. - - Sends simulated HTTP request to Pecan test app. - - :param path: url path of target service - :param params: content for wsgi.input of request - :param expect_errors: Boolean value; whether an error is expected based - on request - :param headers: a dictionary of headers to send along with the request - :param method: Request method type. Appropriate method function call - should be used rather than passing attribute in. - :param extra_environ: a dictionary of environ variables to send along - with the request - :param status: expected status code of response - :param path_prefix: prefix of the url path - """ - full_path = path_prefix + path - response = getattr(self.app, "%s_json" % method)( - str(full_path), - params=params, - headers=headers, - status=status, - extra_environ=extra_environ, - expect_errors=expect_errors - ) - return response - - def put_json(self, path, params, expect_errors=False, headers=None, - extra_environ=None, status=None): - """Simulate an put request. - - Sends simulated HTTP PUT request to Pecan test app. - - :param path: url path of target service - :param params: content for wsgi.input of request - :param expect_errors: Boolean value; whether an error is expected based - on request - :param headers: a dictionary of headers to send along with the request - :param extra_environ: a dictionary of environ variables to send along - with the request - :param status: expected status code of response - """ - return self._request_json(path=path, params=params, - expect_errors=expect_errors, - headers=headers, extra_environ=extra_environ, - status=status, method="put") - - def post_json(self, path, params, expect_errors=False, headers=None, - extra_environ=None, status=None): - """Simulate an post request. - - Sends simulated HTTP POST request to Pecan test app. - - :param path: url path of target service - :param params: content for wsgi.input of request - :param expect_errors: Boolean value; whether an error is expected based - on request - :param headers: a dictionary of headers to send along with the request - :param extra_environ: a dictionary of environ variables to send along - with the request - :param status: expected status code of response - """ - return self._request_json(path=path, params=params, - expect_errors=expect_errors, - headers=headers, extra_environ=extra_environ, - status=status, method="post") - - def patch_json(self, path, params, expect_errors=False, headers=None, - extra_environ=None, status=None): - """Simulate an patch request. - - Sends simulated HTTP PATCH request to Pecan test app. - - :param path: url path of target service - :param params: content for wsgi.input of request - :param expect_errors: Boolean value; whether an error is expected based - on request - :param headers: a dictionary of headers to send along with the request - :param extra_environ: a dictionary of environ variables to send along - with the request - :param status: expected status code of response - """ - return self._request_json(path=path, params=params, - expect_errors=expect_errors, - headers=headers, extra_environ=extra_environ, - status=status, method="patch") - - def delete_json(self, path, params, expect_errors=False, headers=None, - extra_environ=None, status=None, path_prefix=PATH_PREFIX): - """Simulate a delete request. - - Sends simulated HTTP DELETE request to Pecan test app. - - :param path: url path of target service - :param params: content for wsgi.input of request - :param expect_errors: Boolean value; whether an error is expected based - on request - :param headers: a dictionary of headers to send along with the request - :param extra_environ: a dictionary of environ variables to send along - with the request - :param status: expected status code of response - :param path_prefix: prefix of the url path - """ - return self._request_json(path=path, params=params, - expect_errors=expect_errors, - headers=headers, extra_environ=extra_environ, - status=status, method="delete") - - def get_json(self, path, expect_errors=False, headers=None, - extra_environ=None, q=[], path_prefix=PATH_PREFIX, **params): - """Simulate a get request. - - Sends simulated HTTP GET request to Pecan test app. - - :param path: url path of target service - :param expect_errors: Boolean value;whether an error is expected based - on request - :param headers: a dictionary of headers to send along with the request - :param extra_environ: a dictionary of environ variables to send along - with the request - :param q: list of queries consisting of: field, value, op, and type - keys - :param path_prefix: prefix of the url path - :param params: content for wsgi.input of request - """ - full_path = path_prefix + path - query_params = {"q.field": [], - "q.value": [], - "q.op": [], - } - for query in q: - for name in ["field", "op", "value"]: - query_params["q.%s" % name].append(query.get(name, "")) - all_params = {} - all_params.update(params) - if q: - all_params.update(query_params) - response = self.app.get(full_path, - params=all_params, - headers=headers, - extra_environ=extra_environ, - expect_errors=expect_errors) - if not expect_errors: - response = response.json - return response - - def validate_link(self, link, bookmark=False): - """Check if the given link can get correct data.""" - # removes the scheme and net location parts of the link - url_parts = list(parse.urlparse(link)) - url_parts[0] = url_parts[1] = "" - - # bookmark link should not have the version in the URL - if bookmark and url_parts[2].startswith(PATH_PREFIX): - return False - - full_path = parse.urlunparse(url_parts) - try: - self.get_json(full_path, path_prefix="") - return True - except Exception: - return False diff --git a/tests/unit/aas/rest/controllers/test_root.py b/tests/unit/aas/rest/controllers/test_root.py deleted file mode 100644 index 1f55e442b6..0000000000 --- a/tests/unit/aas/rest/controllers/test_root.py +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright 2014 Kylin Cloud -# All Rights Reserved. -# -# 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. - -from __future__ import print_function - -from tests.unit.aas.rest import base - - -class TestRoot(base.PecanControllerTest): - - def test_get_root(self): - data = self.get_json("/", path_prefix="") - self.assertEqual("v1", data["versions"][0]["id"]) - # Check fields are not empty - [self.assertTrue(f) for f in data.keys()] - - -class TestV1Root(base.PecanControllerTest): - - def test_get_v1_root(self): - data = self.get_json("/") - self.assertEqual("v1", data["id"]) - # Check if all known resources are present and there are no extra ones. - expected_resources = set(["id", "links", "media_types", "status", - "updated_at"]) - actual_resources = set(data.keys()) - # TODO(lyj): There are still no resources in api, we need to add the - # related resources here when new api resources added. - self.assertEqual(expected_resources, actual_resources) - - self.assertIn({"type": "application/vnd.openstack.rally.v1+json", - "base": "application/json"}, data["media_types"]) diff --git a/tests/unit/common/test_utils.py b/tests/unit/common/test_utils.py index a2b2772ef1..7c7269b4ee 100644 --- a/tests/unit/common/test_utils.py +++ b/tests/unit/common/test_utils.py @@ -132,18 +132,6 @@ class ImportModulesTestCase(test.TestCase): utils.try_append_module("rally.common.version", modules) self.assertIn("rally.common.version", modules) - def test_try_append_broken_module(self): - modules = {} - self.assertRaises(ImportError, - utils.try_append_module, - "tests.unit.fixtures.import.broken", - modules) - - def test_import_modules_from_package(self): - utils.import_modules_from_package("tests.unit.fixtures.import.package") - self.assertIn("tests.unit.fixtures.import.package.a", sys.modules) - self.assertIn("tests.unit.fixtures.import.package.b", sys.modules) - class LogTestCase(test.TestCase): diff --git a/tests/unit/fixtures/__init__.py b/tests/unit/fixtures/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/unit/fixtures/import/__init__.py b/tests/unit/fixtures/import/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/unit/fixtures/import/broken.py b/tests/unit/fixtures/import/broken.py deleted file mode 100644 index 34a04c145c..0000000000 --- a/tests/unit/fixtures/import/broken.py +++ /dev/null @@ -1,5 +0,0 @@ -'''This module is broken and cannot be imported. -''' - - -import missing.module.fromnowhere # noqa diff --git a/tests/unit/fixtures/import/package/__init__.py b/tests/unit/fixtures/import/package/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/unit/fixtures/import/package/a.py b/tests/unit/fixtures/import/package/a.py deleted file mode 100644 index 284079e499..0000000000 --- a/tests/unit/fixtures/import/package/a.py +++ /dev/null @@ -1,6 +0,0 @@ -'''Usual module. -''' - - -class Bazz: - pass diff --git a/tests/unit/fixtures/import/package/b.py b/tests/unit/fixtures/import/package/b.py deleted file mode 100644 index 58075714b2..0000000000 --- a/tests/unit/fixtures/import/package/b.py +++ /dev/null @@ -1,7 +0,0 @@ -'''Normal module that can be imported successfully. -''' - - -class Bar: - def __init__(self, foo): - self._foo = foo diff --git a/tests/unit/test_hacking.py b/tests/unit/test_hacking.py index b94c2c57a1..44ee8aeb6c 100644 --- a/tests/unit/test_hacking.py +++ b/tests/unit/test_hacking.py @@ -29,6 +29,17 @@ class HackingTestCase(test.TestCase): self.assertIsNone(method) self.assertIsNone(obj) + def test_skip_ignored_lines(self): + + @checks.skip_ignored_lines + def any_gen(logical_line, file_name): + yield 42 + + self.assertEqual([], list(any_gen("fdafadfdas # noqa", "f"))) + self.assertEqual([], list(any_gen(" # fdafadfdas", "f"))) + self.assertEqual([], list(any_gen(" ", "f"))) + self.assertEqual(42, next(any_gen("otherstuff", "f"))) + def test_correct_usage_of_assert_from_mock(self): correct_method_names = ["assert_any_call", "assert_called_once_with", "assert_called_with", "assert_has_calls"] @@ -61,6 +72,14 @@ class HackingTestCase(test.TestCase): self.assertEqual(4, actual_number) self.assertTrue(actual_msg.startswith("N303")) + def _assert_good_samples(self, checker, samples, module_file="f"): + for s in samples: + self.assertEqual([], list(checker(s, module_file)), s) + + def _assert_bad_samples(self, checker, samples, module_file="f"): + for s in samples: + self.assertEqual(1, len(list(checker(s, module_file))), s) + def test_check_wrong_logging_import(self): bad_imports = ["from oslo_log import log", "import oslo_log", @@ -84,284 +103,145 @@ class HackingTestCase(test.TestCase): self.assertEqual([], list(checkres)) def test_no_translate_debug_logs(self): - self.assertEqual(len(list(checks.no_translate_debug_logs( - "LOG.debug(_('foo'))"))), 1) - self.assertEqual(len(list(checks.no_translate_debug_logs( - "LOG.debug('foo')"))), 0) - - self.assertEqual(len(list(checks.no_translate_debug_logs( - "LOG.info(_('foo'))"))), 0) + bad_samples = ["LOG.debug(_('foo'))"] + self._assert_bad_samples(checks.no_translate_debug_logs, bad_samples) + good_samples = ["LOG.debug('foo')", "LOG.info(_('foo'))"] + self._assert_good_samples(checks.no_translate_debug_logs, good_samples) def test_no_use_conf_debug_check(self): - self.assertEqual(len(list(checks.no_use_conf_debug_check( - "if CONF.debug:", "fakefile"))), 1) + bad_samples = [ + "if CONF.debug:", + "if cfg.CONF.debug" + ] + self._assert_bad_samples(checks.no_use_conf_debug_check, bad_samples) - self.assertEqual(len(list(checks.no_use_conf_debug_check( - "if cfg.CONF.debug", "fakefile"))), 1) - - self.assertEqual(len(list(checks.no_use_conf_debug_check( - "if logging.is_debug()", "fakefile"))), 0) + good_samples = ["if logging.is_debug()"] + self._assert_good_samples(checks.no_use_conf_debug_check, good_samples) def test_assert_true_instance(self): self.assertEqual(len(list(checks.assert_true_instance( "self.assertTrue(isinstance(e, " - "exception.BuildAbortException))"))), 1) + "exception.BuildAbortException))", "f"))), 1) self.assertEqual( - len(list(checks.assert_true_instance("self.assertTrue()"))), 0) + 0, + len(list(checks.assert_true_instance("self.assertTrue()", "f")))) def test_assert_equal_type(self): self.assertEqual(len(list(checks.assert_equal_type( - "self.assertEqual(type(als['QuicAssist']), list)"))), 1) + "self.assertEqual(type(als['QuicAssist']), list)", "f"))), 1) self.assertEqual( - len(list(checks.assert_equal_type("self.assertTrue()"))), 0) - - def test_check_iteritems_method(self): - self.assertEqual(len(list(checks.check_iteritems_method( - "dict.iteritems()"))), 1) - - self.assertEqual(len(list(checks.check_iteritems_method( - "iteritems(dict)"))), 0) - - self.assertEqual(len(list(checks.check_iteritems_method( - "dict.items()"))), 0) - - def test_check_concatenate_dict_with_plus_operand(self): - self.assertEqual(len(list( - checks.check_concatenate_dict_with_plus_operand( - "dict1.items() + dict2.items()"))), 1) - - self.assertEqual(len(list( - checks.check_concatenate_dict_with_plus_operand( - "dict(self.endpoint.to_dict().items() + " - "endpoint.items())"))), 1) - - self.assertEqual(len(list( - checks.check_concatenate_dict_with_plus_operand( - "dict(self.endpoint.to_dict().items() + " - "endpoint.items() + something.items())"))), 1) - - self.assertEqual(len(list( - checks.check_concatenate_dict_with_plus_operand( - "dict1.item() + dict2.item()"))), 0) - - self.assertEqual(len(list( - checks.check_concatenate_dict_with_plus_operand( - "obj.getitems() + obj.getitems()"))), 0) - - self.assertEqual(len(list( - checks.check_concatenate_dict_with_plus_operand( - "obj.getitems() + dict2.items()"))), 0) - - def test_check_basestring_method(self): - self.assertEqual(len(list(checks.check_basestring_method( - "basestring"))), 1) - - self.assertEqual(len(list(checks.check_basestring_method( - "six.string_types"))), 0) - - def test_check_StringIO_method(self): - self.assertEqual(len(list(checks.check_StringIO_method( - "StringIO.StringIO()"))), 1) - - self.assertEqual(len(list(checks.check_StringIO_method( - "six.moves.StringIO()"))), 0) - - def test_check_urlparse_method(self): - self.assertEqual(len(list(checks.check_urlparse_method( - "urlparse.urlparse(url)"))), 1) - - self.assertEqual(len(list(checks.check_urlparse_method( - "six.moves.urllib.parse.urlparse(url)"))), 0) - - def test_check_itertools_imap_method(self): - self.assertEqual(len(list(checks.check_itertools_imap_method( - "itertools.imap()"))), 1) - - self.assertEqual(len(list(checks.check_itertools_imap_method( - "six.moves.map()"))), 0) - - def test_check_xrange_imap_method(self): - self.assertEqual(len(list(checks.check_xrange_method( - "xrange()"))), 1) - - self.assertEqual(len(list(checks.check_xrange_method( - "six.moves.range()"))), 0) - - def test_check_string_lower_upper_case_method(self): - self.assertEqual(len(list(checks.check_string_lower_upper_case_method( - "string.lowercase[:16]"))), 1) - - self.assertEqual(len(list(checks.check_string_lower_upper_case_method( - "string.uppercase[:16]"))), 1) - - self.assertEqual(len(list(checks.check_string_lower_upper_case_method( - "string.ascii_lowercase[:16]"))), 0) - - self.assertEqual(len(list(checks.check_string_lower_upper_case_method( - "string.ascii_uppercase[:16]"))), 0) - - def test_check_next_on_iterator_method(self): - self.assertEqual(len(list(checks.check_next_on_iterator_method( - "iterator.next()"))), 1) - - self.assertEqual(len(list(checks.check_next_on_iterator_method( - "next(iterator)"))), 0) + len(list(checks.assert_equal_type("self.assertTrue()", "f"))), 0) def test_assert_equal_none(self): self.assertEqual(len(list(checks.assert_equal_none( - "self.assertEqual(A, None)"))), 1) + "self.assertEqual(A, None)", "f"))), 1) self.assertEqual(len(list(checks.assert_equal_none( - "self.assertEqual(None, A)"))), 1) + "self.assertEqual(None, A)", "f"))), 1) self.assertEqual( - len(list(checks.assert_equal_none("self.assertIsNone()"))), 0) + len(list(checks.assert_equal_none("self.assertIsNone()", "f"))), 0) def test_assert_true_or_false_with_in_or_not_in(self): - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A in B)"))), 1) + good_lines = [ + "self.assertTrue(any(A > 5 for A in B))", + "self.assertTrue(any(A > 5 for A in B), 'some message')", + "self.assertFalse(some in list1 and some2 in list2)" + ] + self._assert_good_samples(checks.assert_true_or_false_with_in, + good_lines) - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertFalse(A in B)"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A not in B)"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertFalse(A not in B)"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A in B, 'some message')"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertFalse(A in B, 'some message')"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A not in B, 'some message')"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertFalse(A not in B, 'some message')"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A in 'some string with spaces')"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A in 'some string with spaces')"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A in ['1', '2', '3'])"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(A in [1, 2, 3])"))), 1) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(any(A > 5 for A in B))"))), 0) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertTrue(any(A > 5 for A in B), 'some message')"))), 0) - - self.assertEqual(len(list(checks.assert_true_or_false_with_in( - "self.assertFalse(some in list1 and some2 in list2)"))), 0) + bad_lines = [ + "self.assertTrue(A in B)", + "self.assertFalse(A in B)", + "self.assertTrue(A not in B)", + "self.assertFalse(A not in B)", + "self.assertTrue(A in B, 'some message')", + "self.assertFalse(A in B, 'some message')", + "self.assertTrue(A not in B, 'some message')", + "self.assertFalse(A not in B, 'some message')", + "self.assertTrue(A in 'some string with spaces')", + "self.assertTrue(A in 'some string with spaces')", + "self.assertTrue(A in ['1', '2', '3'])", + "self.assertTrue(A in [1, 2, 3])" + ] + self._assert_bad_samples(checks.assert_true_or_false_with_in, + bad_lines) def test_assert_equal_in(self): - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(a in b, True)"))), 1) + good_lines = [ + "self.assertEqual(any(a==1 for a in b), True)", + "self.assertEqual(True, any(a==1 for a in b))", + "self.assertEqual(any(a==1 for a in b), False)", + "self.assertEqual(False, any(a==1 for a in b))" + ] + self._assert_good_samples(checks.assert_equal_in, good_lines) - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(a not in b, True)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual('str' in 'string', True)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual('str' not in 'string', True)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(any(a==1 for a in b), True)"))), 0) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(True, a in b)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(True, a not in b)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(True, 'str' in 'string')"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(True, 'str' not in 'string')"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(True, any(a==1 for a in b))"))), 0) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(a in b, False)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(a not in b, False)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual('str' in 'string', False)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual('str' not in 'string', False)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(any(a==1 for a in b), False)"))), 0) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(False, a in b)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(False, a not in b)"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(False, 'str' in 'string')"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(False, 'str' not in 'string')"))), 1) - - self.assertEqual(len(list(checks.assert_equal_in( - "self.assertEqual(False, any(a==1 for a in b))"))), 0) + bad_lines = [ + "self.assertEqual(a in b, True)", + "self.assertEqual(a not in b, True)", + "self.assertEqual('str' in 'string', True)", + "self.assertEqual('str' not in 'string', True)", + "self.assertEqual(True, a in b)", + "self.assertEqual(True, a not in b)", + "self.assertEqual(True, 'str' in 'string')", + "self.assertEqual(True, 'str' not in 'string')", + "self.assertEqual(a in b, False)", + "self.assertEqual(a not in b, False)", + "self.assertEqual('str' in 'string', False)", + "self.assertEqual('str' not in 'string', False)", + "self.assertEqual(False, a in b)", + "self.assertEqual(False, a not in b)", + "self.assertEqual(False, 'str' in 'string')", + "self.assertEqual(False, 'str' not in 'string')", + ] + self._assert_bad_samples(checks.assert_equal_in, bad_lines) def test_check_no_direct_rally_objects_import(self): - bad_imports = ["from rally.objects import task", "import rally.objects.task"] - good_import = "from rally import objects" + self._assert_bad_samples(checks.check_no_direct_rally_objects_import, + bad_imports) - for bad_import in bad_imports: - checkres = checks.check_no_direct_rally_objects_import(bad_import, - "fakefile") - self.assertIsNotNone(next(checkres)) + self._assert_good_samples(checks.check_no_direct_rally_objects_import, + bad_imports, + module_file="./rally/objects/__init__.py") - for bad_import in bad_imports: - checkres = checks.check_no_direct_rally_objects_import( - bad_import, "./rally/objects/__init__.py") - self.assertEqual([], list(checkres)) - - checkres = checks.check_no_direct_rally_objects_import(good_import, - "fakefile") - self.assertEqual([], list(checkres)) + good_imports = ["from rally import objects"] + self._assert_good_samples(checks.check_no_direct_rally_objects_import, + good_imports) def test_check_no_oslo_deprecated_import(self): - bad_imports = ["from oslo.config", - "import oslo.config," + "import oslo.config", "from oslo.db", - "import oslo.db," + "import oslo.db", "from oslo.i18n", - "import oslo.i18n," + "import oslo.i18n", "from oslo.serialization", - "import oslo.serialization," + "import oslo.serialization", "from oslo.utils", - "import oslo.utils,"] + "import oslo.utils"] - for bad_import in bad_imports: - checkres = checks.check_no_oslo_deprecated_import(bad_import, - "fakefile") - self.assertIsNotNone(next(checkres)) + self._assert_bad_samples(checks.check_no_oslo_deprecated_import, + bad_imports) + + def test_check_quotas(self): + bad_lines = [ + "a = '1'", + "a = \"a\" + 'a'", + "'", + "\"\"\"\"\"\" + ''''''" + ] + self._assert_bad_samples(checks.check_quotes, bad_lines) + + good_lines = [ + "\"'a'\" + \"\"\"a'''fdfd'''\"\"\"", + "\"fdfdfd\" + \"''''''\"", + "a = '' # noqa " + ] + self._assert_good_samples(checks.check_quotes, good_lines)