diff --git a/HACKING.rst b/HACKING.rst index 7ac9b75bf..19c7d9d3a 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -12,3 +12,4 @@ Horizon Specific Commandments ----------------------------- - Read the Horizon contributing documentation at http://docs.openstack.org/developer/horizon/contributing.html +- [M322] Method's default argument shouldn't be mutable. diff --git a/horizon/hacking/__init__.py b/horizon/hacking/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/horizon/hacking/checks.py b/horizon/hacking/checks.py new file mode 100644 index 000000000..4a61166f8 --- /dev/null +++ b/horizon/hacking/checks.py @@ -0,0 +1,41 @@ +# Copyright (c) 2015 Intel, Inc. +# 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. + +import re + +""" +Guidelines for writing new hacking checks + - Use only for Horizon specific tests. OpenStack general tests + should be submitted to the common 'hacking' module. + - Pick numbers in the range M3xx. Find the current test with + the highest allocated number and then pick the next value. + If nova has an N3xx code for that test, use the same number. + - Keep the test method code in the source file ordered based + on the M3xx 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 +""" + +mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") + + +def no_mutable_default_args(logical_line): + msg = "M322: Method's default argument shouldn't be mutable!" + if mutable_default_args.match(logical_line): + yield (0, msg) + + +def factory(register): + register(no_mutable_default_args) diff --git a/horizon/test/test_hacking.py b/horizon/test/test_hacking.py new file mode 100644 index 000000000..32b9fd0ac --- /dev/null +++ b/horizon/test/test_hacking.py @@ -0,0 +1,83 @@ +# Copyright 2015 Intel, Inc. +# +# 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 mock +import pep8 +import textwrap + +from horizon.hacking import checks +from horizon.test import helpers + + +class HackingTestCase(helpers.TestCase): + """This class tests the hacking checks in horizon.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 _assert_has_no_errors(self, code, checker, filename=None): + self._assert_has_errors(code, checker, filename=filename) + + def test_no_mutable_default_args(self): + self.assertEqual(1, len(list(checks.no_mutable_default_args( + "def get_info_from_bdm(virt_type, bdm, mapping=[])")))) + + self.assertEqual(0, len(list(checks.no_mutable_default_args( + "defined = []")))) + + self.assertEqual(0, len(list(checks.no_mutable_default_args( + "defined, undefined = [], {}")))) diff --git a/openstack_dashboard/api/glance.py b/openstack_dashboard/api/glance.py index 7f21b3161..1787ab17c 100644 --- a/openstack_dashboard/api/glance.py +++ b/openstack_dashboard/api/glance.py @@ -242,7 +242,7 @@ def metadefs_namespace_get(request, namespace, resource_type=None, wrap=False): def metadefs_namespace_list(request, - filters={}, + filters=None, sort_dir='asc', sort_key='namespace', marker=None, @@ -275,6 +275,8 @@ def metadefs_namespace_list(request, if get_version() < 2: return [], False, False + if filters is None: + filters = {} limit = getattr(settings, 'API_RESULT_LIMIT', 1000) page_size = utils.get_page_size(request) diff --git a/openstack_dashboard/test/helpers.py b/openstack_dashboard/test/helpers.py index ae05a62b0..ce4c0f399 100644 --- a/openstack_dashboard/test/helpers.py +++ b/openstack_dashboard/test/helpers.py @@ -58,7 +58,7 @@ from openstack_dashboard.test.test_data import utils as test_utils wsgi.WSGIRequest.__repr__ = lambda self: "" -def create_stubs(stubs_to_create={}): +def create_stubs(stubs_to_create=None): """decorator to simplify setting up multiple stubs at once via mox :param stubs_to_create: methods to stub in one or more modules @@ -89,7 +89,8 @@ def create_stubs(stubs_to_create={}): } """ - + if stubs_to_create is None: + stubs_to_create = {} if not isinstance(stubs_to_create, dict): raise TypeError("create_stub must be passed a dict, but a %s was " "given." % type(stubs_to_create).__name__) diff --git a/tox.ini b/tox.ini index 875adf5b5..ae3015720 100644 --- a/tox.ini +++ b/tox.ini @@ -111,6 +111,7 @@ ignore = H405 max-complexity = 20 [hacking] +local-check-factory = horizon.hacking.checks.factory import_exceptions = collections.defaultdict, collections.OrderedDict, django.conf.settings,