Merge "Fix "mutable" object as default value"
diff --git a/HACKING.rst b/HACKING.rst
index 8652971..025bf74 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -12,6 +12,7 @@
- [T104] Scenario tests require a services decorator
- [T105] Unit tests cannot use setUpClass
- [T106] vim configuration should not be kept in source files.
+- [N322] Method's default argument shouldn't be mutable
Test Data/Configuration
-----------------------
diff --git a/tempest/api/compute/base.py b/tempest/api/compute/base.py
index 343a39a..47d1254 100644
--- a/tempest/api/compute/base.py
+++ b/tempest/api/compute/base.py
@@ -271,10 +271,10 @@
return resp, body
@classmethod
- def create_test_server_group(cls, name="", policy=[]):
+ def create_test_server_group(cls, name="", policy=None):
if not name:
name = data_utils.rand_name(cls.__name__ + "-Server-Group")
- if not policy:
+ if policy is None:
policy = ['affinity']
resp, body = cls.servers_client.create_server_group(name, policy)
cls.server_groups.append(body['id'])
diff --git a/tempest/api/orchestration/base.py b/tempest/api/orchestration/base.py
index d0fb825..0b22de5 100644
--- a/tempest/api/orchestration/base.py
+++ b/tempest/api/orchestration/base.py
@@ -64,8 +64,10 @@
return admin_client
@classmethod
- def create_stack(cls, stack_name, template_data, parameters={},
+ def create_stack(cls, stack_name, template_data, parameters=None,
environment=None, files=None):
+ if parameters is None:
+ parameters = {}
resp, body = cls.client.create_stack(
stack_name,
template=template_data,
diff --git a/tempest/common/rest_client.py b/tempest/common/rest_client.py
index ff92b67..132d0a6 100644
--- a/tempest/common/rest_client.py
+++ b/tempest/common/rest_client.py
@@ -248,8 +248,10 @@
return resp[i]
return ""
- def _log_request_start(self, method, req_url, req_headers={},
+ def _log_request_start(self, method, req_url, req_headers=None,
req_body=None):
+ if req_headers is None:
+ req_headers = {}
caller_name = misc_utils.find_test_caller()
trace_regex = CONF.debug.trace_requests
if trace_regex and re.search(trace_regex, caller_name):
@@ -257,8 +259,10 @@
(caller_name, method, req_url))
def _log_request(self, method, req_url, resp,
- secs="", req_headers={},
+ secs="", req_headers=None,
req_body=None, resp_body=None):
+ if req_headers is None:
+ req_headers = {}
# if we have the request id, put it in the right part of the log
extra = dict(request_id=self._get_request_id(resp))
# NOTE(sdague): while we still have 6 callers to this function
diff --git a/tempest/hacking/checks.py b/tempest/hacking/checks.py
index cef010e..abc60cb 100644
--- a/tempest/hacking/checks.py
+++ b/tempest/hacking/checks.py
@@ -27,6 +27,7 @@
SETUPCLASS_DEFINITION = re.compile(r'^\s*def setUpClass')
SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)')
VI_HEADER_RE = re.compile(r"^#\s+vim?:.+")
+mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
def import_no_clients_in_api(physical_line, filename):
@@ -119,6 +120,16 @@
'tests')
+def no_mutable_default_args(logical_line):
+ """Check that mutable object isn't used as default argument
+
+ N322: Method's default argument shouldn't be mutable
+ """
+ msg = "N322: Method's default argument shouldn't be mutable!"
+ if mutable_default_args.match(logical_line):
+ yield (0, msg)
+
+
def factory(register):
register(import_no_clients_in_api)
register(scenario_tests_need_service_tags)
@@ -126,3 +137,4 @@
register(no_vi_headers)
register(service_tags_not_in_module_path)
register(no_official_client_manager_in_api_tests)
+ register(no_mutable_default_args)
diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py
index 0f14c94..c7236ce 100644
--- a/tempest/scenario/manager.py
+++ b/tempest/scenario/manager.py
@@ -136,8 +136,8 @@
pass
def addCleanup_with_wait(self, waiter_callable, thing_id, thing_id_param,
- cleanup_callable, cleanup_args=[],
- cleanup_kwargs={}, ignore_error=True):
+ cleanup_callable, cleanup_args=None,
+ cleanup_kwargs=None, ignore_error=True):
"""Adds wait for ansyc resource deletion at the end of cleanups
@param waiter_callable: callable to wait for the resource to delete
@@ -147,6 +147,10 @@
the following *cleanup_args, **cleanup_kwargs.
usually a delete method.
"""
+ if cleanup_args is None:
+ cleanup_args = []
+ if cleanup_kwargs is None:
+ cleanup_kwargs = {}
self.addCleanup(cleanup_callable, *cleanup_args, **cleanup_kwargs)
wait_dict = {
'waiter_callable': waiter_callable,
@@ -181,7 +185,7 @@
def create_server(self, name=None, image=None, flavor=None,
wait_on_boot=True, wait_on_delete=True,
- create_kwargs={}):
+ create_kwargs=None):
"""Creates VM instance.
@param image: image from which to create the instance
@@ -196,6 +200,8 @@
image = CONF.compute.image_ref
if flavor is None:
flavor = CONF.compute.flavor_ref
+ if create_kwargs is None:
+ create_kwargs = {}
fixed_network_name = CONF.compute.fixed_network_name
if 'nics' not in create_kwargs and fixed_network_name:
@@ -338,7 +344,9 @@
return linux_client
- def _image_create(self, name, fmt, path, properties={}):
+ def _image_create(self, name, fmt, path, properties=None):
+ if properties is None:
+ properties = {}
name = data_utils.rand_name('%s-' % name)
image_file = open(path, 'rb')
self.addCleanup(image_file.close)
@@ -531,8 +539,8 @@
def addCleanup_with_wait(self, things, thing_id,
error_status='ERROR',
exc_type=nova_exceptions.NotFound,
- cleanup_callable=None, cleanup_args=[],
- cleanup_kwargs={}):
+ cleanup_callable=None, cleanup_args=None,
+ cleanup_kwargs=None):
"""Adds wait for ansyc resource deletion at the end of cleanups
@param things: type of the resource to delete
@@ -544,6 +552,10 @@
usually a delete method. if not used, will try to use:
things.delete(thing_id)
"""
+ if cleanup_args is None:
+ cleanup_args = []
+ if cleanup_kwargs is None:
+ cleanup_kwargs = {}
if cleanup_callable is None:
LOG.debug("no delete method passed. using {rclass}.delete({id}) as"
" default".format(rclass=things, id=thing_id))
@@ -725,7 +737,7 @@
def create_server(self, client=None, name=None, image=None, flavor=None,
wait_on_boot=True, wait_on_delete=True,
- create_kwargs={}):
+ create_kwargs=None):
"""Creates VM instance.
@param client: compute client to create the instance
@@ -743,6 +755,8 @@
image = CONF.compute.image_ref
if flavor is None:
flavor = CONF.compute.flavor_ref
+ if create_kwargs is None:
+ create_kwargs = {}
fixed_network_name = CONF.compute.fixed_network_name
if 'nics' not in create_kwargs and fixed_network_name:
@@ -872,7 +886,9 @@
self.status_timeout(
self.volume_client.volumes, volume_id, status)
- def _image_create(self, name, fmt, path, properties={}):
+ def _image_create(self, name, fmt, path, properties=None):
+ if properties is None:
+ properties = {}
name = data_utils.rand_name('%s-' % name)
image_file = open(path, 'rb')
self.addCleanup(image_file.close)
@@ -1866,12 +1882,17 @@
self._list_and_check_container_objects(container_name,
not_present_obj=[filename])
- def _list_and_check_container_objects(self, container_name, present_obj=[],
- not_present_obj=[]):
+ def _list_and_check_container_objects(self, container_name,
+ present_obj=None,
+ not_present_obj=None):
"""
List objects for a given container and assert which are present and
which are not.
"""
+ if present_obj is None:
+ present_obj = []
+ if not_present_obj is None:
+ not_present_obj = []
_, object_list = self.container_client.list_container_contents(
container_name)
if present_obj:
diff --git a/tempest/services/object_storage/account_client.py b/tempest/services/object_storage/account_client.py
index be0f888..eca57c0 100644
--- a/tempest/services/object_storage/account_client.py
+++ b/tempest/services/object_storage/account_client.py
@@ -32,11 +32,15 @@
def create_account(self, data=None,
params=None,
- metadata={},
- remove_metadata={},
+ metadata=None,
+ remove_metadata=None,
metadata_prefix='X-Account-Meta-',
remove_metadata_prefix='X-Remove-Account-Meta-'):
"""Create an account."""
+ if metadata is None:
+ metadata = {}
+ if remove_metadata is None:
+ remove_metadata = {}
url = ''
if params:
url += '?%s' % urllib.urlencode(params)
diff --git a/tempest/services/orchestration/json/orchestration_client.py b/tempest/services/orchestration/json/orchestration_client.py
index 9c76f51..d3867cd 100644
--- a/tempest/services/orchestration/json/orchestration_client.py
+++ b/tempest/services/orchestration/json/orchestration_client.py
@@ -45,9 +45,11 @@
body = json.loads(body)
return resp, body['stacks']
- def create_stack(self, name, disable_rollback=True, parameters={},
+ def create_stack(self, name, disable_rollback=True, parameters=None,
timeout_mins=60, template=None, template_url=None,
environment=None, files=None):
+ if parameters is None:
+ parameters = {}
headers, body = self._prepare_update_create(
name,
disable_rollback,
@@ -63,8 +65,10 @@
return resp, body
def update_stack(self, stack_identifier, name, disable_rollback=True,
- parameters={}, timeout_mins=60, template=None,
+ parameters=None, timeout_mins=60, template=None,
template_url=None, environment=None, files=None):
+ if parameters is None:
+ parameters = {}
headers, body = self._prepare_update_create(
name,
disable_rollback,
@@ -80,9 +84,11 @@
return resp, body
def _prepare_update_create(self, name, disable_rollback=True,
- parameters={}, timeout_mins=60,
+ parameters=None, timeout_mins=60,
template=None, template_url=None,
environment=None, files=None):
+ if parameters is None:
+ parameters = {}
post_body = {
"stack_name": name,
"disable_rollback": disable_rollback,
@@ -264,16 +270,20 @@
body = json.loads(body)
return resp, body
- def validate_template(self, template, parameters={}):
+ def validate_template(self, template, parameters=None):
"""Returns the validation result for a template with parameters."""
+ if parameters is None:
+ parameters = {}
post_body = {
'template': template,
'parameters': parameters,
}
return self._validate_template(post_body)
- def validate_template_url(self, template_url, parameters={}):
+ def validate_template_url(self, template_url, parameters=None):
"""Returns the validation result for a template with parameters."""
+ if parameters is None:
+ parameters = {}
post_body = {
'template_url': template_url,
'parameters': parameters,
diff --git a/tempest/tests/test_hacking.py b/tempest/tests/test_hacking.py
index 52fdf7e..9c13013 100644
--- a/tempest/tests/test_hacking.py
+++ b/tempest/tests/test_hacking.py
@@ -107,3 +107,16 @@
self.assertFalse(checks.no_official_client_manager_in_api_tests(
"cls.official_client = clients.OfficialClientManager(credentials)",
"tempest/scenario/fake_test.py"))
+
+ def test_no_mutable_default_args(self):
+ self.assertEqual(1, len(list(checks.no_mutable_default_args(
+ " def function1(para={}):"))))
+
+ self.assertEqual(1, len(list(checks.no_mutable_default_args(
+ "def function2(para1, para2, para3=[])"))))
+
+ self.assertEqual(0, len(list(checks.no_mutable_default_args(
+ "defined = []"))))
+
+ self.assertEqual(0, len(list(checks.no_mutable_default_args(
+ "defined, undefined = [], {}"))))