pylint fixes in configurable.
Removed unused variables, and changes
single line pylint disable flags to
two line disabls flags that return the
line length below 80 chars.
BUG=chromium:272493
TEST=run_all_tests.py
Change-Id: Iabdb7cc2b0a0a35a1ea6e6715f39a452a6b0d292
Reviewed-on: https://chromium-review.googlesource.com/182850
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Byron Kubert <byronk@chromium.org>
Commit-Queue: Byron Kubert <byronk@chromium.org>
diff --git a/self_tests/unittests/test_configurable.py b/self_tests/unit_tests_software/test_configurable.py
similarity index 71%
rename from self_tests/unittests/test_configurable.py
rename to self_tests/unit_tests_software/test_configurable.py
index f67c331..068dddc 100644
--- a/self_tests/unittests/test_configurable.py
+++ b/self_tests/unit_tests_software/test_configurable.py
@@ -2,12 +2,21 @@
# Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+"""
+Children of this class can specify how they need to be configured.
+"""
+# These test methods don't always use self.
+#pylint: disable=no-self-use
+# There are many tests.
+#pylint: disable=too-many-public-methods
+# These test names are very verbose to aid in debugging.
+#pylint: disable=invalid-name
+
import copy
import StringIO
import sys
import unittest
-from pprint import pprint
from wireless_automation.aspects import configurable
from wireless_automation.aspects import wireless_automation_logging
@@ -85,6 +94,9 @@
class ClassWithAllTheConfigTypes(configurable.Configurable):
+ """
+ Test Class with all the types.
+ """
CONFIGSPEC = [
"Number = integer(min=0, max=10, default=9)",
#"Float = float (min='0', max='1.111', default = '1.1')",
@@ -96,12 +108,12 @@
"IntList = int_list(min=1, max=10, default =list(1,2))",
"FloatList = float_list(min=1, max=10, default =list('1.1',2))",
"BoolList = bool_list(min=1, max=10, "
- "default =list(True,False,0,'yes',no))",
+ "default =list(True,False,0,'yes',no))",
"IpAddrList = ip_addr_list(min=1, max=10, "
- "default =list(10.10.10.1,1.1.1.1))",
+ "default =list(10.10.10.1,1.1.1.1))",
"StringList = string_list(min=1, max=10, "
- "default =list('string1', 'string2'))",
- #Mixed list does not have the default keyword, making this
+ "default =list('string1', 'string2'))",
+ # Mixed list does not have the default keyword, making this
# not useful for Configurable, which uses defaults extensively.
#"MixedList = Do not use.
"AlwaysOK = pass(default = None) ",
@@ -121,29 +133,31 @@
Modify a config spec in a derived class.
"""
class NewClass(ConfigurableTestClass1):
+ """
+ Test Class.
+ """
# ConfigObj keeps some deep links to the orig list used
# to construct it. Without the deepcopy, other tests that
# run after this test get the modified data and fail.
base_config = copy.deepcopy(ConfigurableTestClass1.CONFIGSPEC)
base_configspec = configurable.list_to_configspec(base_config)
- base_configspec['level_1']['midnumber'] = 'float'
+ base_configspec['level_1']['midnumber'] = 'float(default=1)'
CONFIGSPEC = base_configspec
- def __init__(self):
- pass
+ def __init__(self, config):
+ ConfigurableTestClass1.__init__(self, config)
- x = NewClass()
- assert x.CONFIGSPEC['level_1']['midnumber'] == 'float'
+ tmp_config = NewClass.get_default_config()
+ assert tmp_config['level_1']['midnumber'] == 1
def test_config_obj_to_string(self):
"""
Can we get the string rep of a
"""
- tt = ConfigurableTestClass1()
- default_config = tt.get_default_config()
+ temp = ConfigurableTestClass1()
+ default_config = temp.get_default_config()
config_str = configurable.configobj_to_string(default_config)
- print config_str
assert '#comment here' in config_str
assert 'flavor = berrydefault'in config_str
@@ -170,26 +184,27 @@
the [] hierarchy
"""
class ManyConfigSpecs(configurable.Configurable):
+ """
+ Class with many different configspecs.
+ """
- CONFIGSPEC = ['#TopTopComment '] + \
- configurable.nest_configspecs([
- ('class1', ConfigurableTestClass1),
- ('class2', ConfigurableTestClass1)]
- )
+ CONFIGSPEC = ['#TopTopComment '] + configurable.nest_configspecs([
+ ('class1', ConfigurableTestClass1),
+ ('class2', ConfigurableTestClass1)]
+ )
- def __init__(self, config):
- super(ManyConfigSpecs, self).__init__(config)
+ def __init__(self, l_config):
+ super(ManyConfigSpecs, self).__init__(l_config)
config = ManyConfigSpecs.get_default_config()
- print "\n".join(config.configspec.write())
- test_class = ManyConfigSpecs(config)
+ ManyConfigSpecs(config)
def test_write_and_read_config_file(self):
"""
test_write_and_read_config_file
"""
- tt = ConfigurableTestClass1()
- default_config = tt.get_default_config()
+ temp = ConfigurableTestClass1()
+ default_config = temp.get_default_config()
fh = StringIO.StringIO()
default_config.write(fh)
fh.seek(0)
@@ -202,46 +217,46 @@
"""
test_pass_part_of_config_to_object
"""
- tt = ConfigurableTestClass1()
+ temp = ConfigurableTestClass1()
sub_configspec = configurable.get_section_of_configspec(
- tt.config.configspec, 'level_1')
+ temp.config.configspec, 'level_1')
ConfigurableOnlyNeedsLevel1.CONFIGSPEC = sub_configspec
- config = tt.get_default_config()
+ config = temp.get_default_config()
section = configurable.get_section_of_config(config, 'level_1')
- tt1 = ConfigurableOnlyNeedsLevel1(section)
+ ConfigurableOnlyNeedsLevel1(section)
def test_get_section_of_configspec_should_raise_on_dict_input(self):
"""
test_get_section_of_configspec_should_raise_on_dict_input
"""
- tt = ConfigurableTestClass1()
+ ConfigurableTestClass1()
with self.assertRaises(configurable.ConfigurationError):
- sub_configspec = configurable.get_section_of_configspec(
+ configurable.get_section_of_configspec(
{'spec': '=integer'}, {'level_1'})
def test_build_config_from_two_classes_and_get_subsections(self):
"""
test_build_config_from_two_classes_and_get_subsections
"""
- tt = ConfigurableTestClass1()
+ temp = ConfigurableTestClass1()
combined = configurable.combine_configs([
- ('first', tt.config),
- ('second', tt.config)
+ ('first', temp.config),
+ ('second', temp.config)
])
combined_list_str = combined.write()
self.assertEqual(combined_list_str[0], '[first]', 'combined lists')
- self.assertEqual(combined['first'], tt.config, '[first] missing')
- self.assertEqual(combined['second'], tt.config, '[second] missing')
+ self.assertEqual(combined['first'], temp.config, '[first] missing')
+ self.assertEqual(combined['second'], temp.config, '[second] missing')
def test_combine_configs_should_raise_on_duplicate_names(self):
"""
test_combine_configs_should_raise_on_duplicate_names
"""
- tt = ConfigurableTestClass1()
+ temp = ConfigurableTestClass1()
with self.assertRaises(configurable.ConfigurationError):
- combined = configurable.combine_configs([
- ('first', tt.config),
- ('first', tt.config)
+ configurable.combine_configs([
+ ('first', temp.config),
+ ('first', temp.config)
])
def test_get_section_of_configspec_should_raise_on_missing_section(self):
@@ -251,7 +266,7 @@
configspec = ConfigurableTestClass1.CONFIGSPEC
section = 'non_existant_section_name'
with self.assertRaises(configurable.ConfigurationError):
- configspec_section = configurable.get_section_of_configspec(
+ configurable.get_section_of_configspec(
configspec, section)
def test_get_section_of_configspec(self):
@@ -271,8 +286,8 @@
"""
test_get_section_of_config
"""
- tt = ConfigurableTestClass1()
- config = tt.get_default_config()
+ temp = ConfigurableTestClass1()
+ config = temp.get_default_config()
sub_section = configurable.get_section_of_config(config, 'level_1')
self.assertEqual(sub_section[0], 'midnumber = 22')
@@ -286,15 +301,15 @@
test_make_config_with_many_types_works
"""
config = ClassWithAllTheConfigTypes.get_default_config()
- a = ClassWithAllTheConfigTypes(config)
+ ClassWithAllTheConfigTypes(config)
def test_configspec_creates_valid_default_config(self):
"""
test_configspec_creates_valid_default_config
"""
- tt = ConfigurableTestClass1()
- default_config = tt.get_default_config()
- self.assertTrue(tt.is_this_config_valid(default_config))
+ temp = ConfigurableTestClass1()
+ default_config = temp.get_default_config()
+ self.assertTrue(temp.is_this_config_valid(default_config))
def add_bad_data_and_check(key, value):
"""
@@ -304,7 +319,7 @@
"""
bad_config = copy.copy(default_config)
bad_config[key] = value
- is_valid = tt.is_this_config_valid(bad_config)
+ is_valid = temp.is_this_config_valid(bad_config)
self.assertFalse(is_valid)
add_bad_data_and_check('topnumber', '111') # Too large
@@ -315,44 +330,47 @@
"""
test_many_different_invalid_config_parts
"""
- tt = ConfigurableTestClass1()
- default_config = tt.get_default_config()
+ temp = ConfigurableTestClass1()
+ default_config = temp.get_default_config()
bad_config = copy.copy(default_config)
bad_config['level_1']['not_there'] = 1212
- self.assertFalse(tt.is_this_config_valid(bad_config))
+ self.assertFalse(temp.is_this_config_valid(bad_config))
bad_config = copy.copy(default_config)
bad_config['level_1']['midnumber'] = 1212
- self.assertFalse(tt.is_this_config_valid(bad_config))
+ self.assertFalse(temp.is_this_config_valid(bad_config))
bad_config = copy.copy(default_config)
bad_config['level_1']['midnumber2'] = 121232
- self.assertFalse(tt.is_this_config_valid(bad_config))
+ self.assertFalse(temp.is_this_config_valid(bad_config))
bad_config = copy.copy(default_config)
bad_config['level_1']['level_22'] = 121232
- self.assertFalse(tt.is_this_config_valid(bad_config))
+ self.assertFalse(temp.is_this_config_valid(bad_config))
bad_config = copy.copy(default_config)
bad_config['level_1']['level_2']['number2'] = 121232
- self.assertFalse(tt.is_this_config_valid(bad_config))
+ self.assertFalse(temp.is_this_config_valid(bad_config))
def test_many_invalid_config_parts_at_once(self):
"""
test_many_invalid_config_parts_at_once
"""
- tt = ConfigurableTestClass1()
- default_config = tt.get_default_config()
+ temp = ConfigurableTestClass1()
+ default_config = temp.get_default_config()
bad_config = copy.copy(default_config)
bad_config['level_1']['not_there'] = 1212
bad_config['level_1']['midnumber'] = 1212
bad_config['level_1']['midnumber2'] = 121232
bad_config['level_1']['level_22'] = 121232
bad_config['level_1']['level_2']['number2'] = 121232
- self.assertFalse(tt.is_this_config_valid(bad_config))
+ self.assertFalse(temp.is_this_config_valid(bad_config))
def test_is_this_config_valid_should_raise_on_bad_input(self):
+ """
+ On bad input, this should raise an exception
+ """
with self.assertRaises(configurable.ConfigurationError):
ConfigurableTestClass1.is_this_config_valid(['bad_type'])
@@ -361,23 +379,29 @@
test_missing_configspec_raises_error
"""
with self.assertRaises(configurable.ConfigurationError):
- tt = ConfigurableTestClassNoConfigspec()
+ ConfigurableTestClassNoConfigspec()
def test_configspec_must_not_be_dict(self):
"""
test_configspec_must_not_be_dict
"""
with self.assertRaises(configurable.ConfigurationError):
- tt = ConfigurableWithConfigspecDict()
+ ConfigurableWithConfigspecDict()
def test_subset_of_config_options_add_to_defaults(self):
"""
test_subset_of_config_options_add_to_defaults
"""
- tt = ConfigurableTestClass1(['topnumber=33'])
- self.assertEqual(tt.config['topnumber'], 33)
- self.assertEqual(tt.config['level_1']['midnumber'], 22)
- tt.is_this_config_valid(tt.config)
+ temp = ConfigurableTestClass1(['topnumber=33'])
+ self.assertEqual(temp.config['topnumber'], 33)
+ self.assertEqual(temp.config['level_1']['midnumber'], 22)
+ temp.is_this_config_valid(temp.config)
+
+
+class TestConfigurableError(unittest.TestCase):
+ """
+ Check for error cases
+ """
def test_get_items_in_config_not_in_configspec(self):
"""
@@ -387,7 +411,7 @@
configspec = ConfigurableTestClass1.CONFIGSPEC
config['extra_junk'] = 33
extras = configurable.get_items_in_config_not_in_configspec(
- configspec, config)
+ configspec, config)
self.assertEqual(extras[0][1], 'extra_junk')
def test_invalid_config_list_raises_error(self):
@@ -395,45 +419,68 @@
test_extra_values_raise_error
"""
with self.assertRaises(configurable.ConfigurationError):
- tt = ConfigurableTestClass1(['extra_thing=33'])
+ ConfigurableTestClass1(['extra_thing=33'])
def test_invalid_config_dict_raises_error(self):
"""
test_invalid_config_raises_error
"""
with self.assertRaises(configurable.ConfigurationError):
- tt = ConfigurableTestClass1({'bad_config_data': 333})
+ ConfigurableTestClass1({'bad_config_data': 333})
def test_init_with_configobj_as_confgspec(self):
"""
test_init_with_configobj_as_confgspec
"""
- tt = ConfigurableClassWithConfigobjAsConfigspec()
+ ConfigurableClassWithConfigobjAsConfigspec()
def test_valid_config(self):
"""
test_valid_config
"""
- tt = ConfigurableTestClass1()
- default_config = tt.get_default_config()
- tt2 = ConfigurableTestClass1(default_config)
+ temp = ConfigurableTestClass1()
+ default_config = temp.get_default_config()
+ ConfigurableTestClass1(default_config)
def test_check_config_raises_error(self):
"""
test_get_default_config_raises_error
"""
with self.assertRaises(configurable.ConfigurationError):
- tt = BrokenConfigurableTestClass1()
+ BrokenConfigurableTestClass1()
def test_get_default_config_raises_error(self):
"""
test_get_default_config_raises_error
"""
with self.assertRaises(configurable.ConfigurationError):
- tt = BrokenConfigurableTestClass1List.get_default_config()
+ BrokenConfigurableTestClass1List.get_default_config()
+
+ def test_get_config(self):
+ """
+ Test get_config. It should read back the config object that
+ was used to init it.
+ """
+ config = ConfigurableTestClass1.get_default_config()
+ temp = ConfigurableTestClass1(config)
+ used_config = temp.get_config()
+ assert used_config == config
+
+ def test_empty_config_makes_default_object(self):
+ """
+ The constructor of a Configurable object uses the incoming
+ config to add to it's config. So you can specify only one
+ parameter and have the rest default. Building an obect
+ with an empty config should produce an object with the
+ default setings.
+ """
+ config = ConfigurableTestClass1.get_default_config()
+ temp = ConfigurableTestClass1(config)
+ temp2 = ConfigurableTestClass1()
+ assert temp2.get_config() == temp.get_config()
if __name__ == '__main__':
- log = wireless_automation_logging.setup_logging(
+ LOG = wireless_automation_logging.setup_logging(
'configurable', output_stream=sys.stdout)
unittest.main(verbosity=2)
diff --git a/wireless_automation/aspects/configurable.py b/wireless_automation/aspects/configurable.py
index 5a38697..cab278e 100644
--- a/wireless_automation/aspects/configurable.py
+++ b/wireless_automation/aspects/configurable.py
@@ -69,7 +69,8 @@
return base_config
-def get_items_in_config_not_in_configspec(configspec, config): # pylint: disable=C0103,C0301
+# pylint: disable=invalid-name
+def get_items_in_config_not_in_configspec(configspec, config):
"""
:param config: ConfigObj
@@ -87,6 +88,7 @@
preserve_errors=True, copy=True)
extra_items = configobj.get_extra_values(new_config)
return extra_items
+# pylint: enable=invalid-name
def configobj_to_string(config):
@@ -133,9 +135,10 @@
try:
return configobj.ConfigObj(configspec=string_list).configspec
except configobj.ConfigspecError, error:
- print (error)
- print ("This Configspec did not parse:")
- print (string_list)
+ log = logging.getLogger(LOG_NAME)
+ log.error(error)
+ log.error("This Configspec did not parse:")
+ log.error(string_list)
raise
@@ -189,7 +192,9 @@
# Convert to a list of strings, like what would be in a file
strings = configspec.write()
# Change all the [] to [[]] to indent them all one more level
- strings = [re.sub('\[(.*)\]', '[[\\1]]', x) for x in strings] # pylint: disable=anomalous-backslash-in-string,line-too-long
+ # pylint: disable=anomalous-backslash-in-string
+ strings = [re.sub('\[(.*)\]', '[[\\1]]', x) for x in strings]
+ # pylint: enable=anomalous-backslash-in-string
# Add the class path to top, so we know where this came from
strings = ['# From Class: ' + the_class.__name__] + strings
# Add the name to the top of the configspec
@@ -456,7 +461,7 @@
"""
# The config spec(a ConfigObject term for the definition of the
# configuration)
- CONFIGSPEC = ['NEVER_SET']
+ CONFIGSPEC = ['DEFAULT_VALUE AS SET IN CONFIGURABLE.PY']
def __init__(self, input_config_data=None):
"""
@@ -480,7 +485,9 @@
raise ConfigurationError(error_message)
# Check the configspec for sanity.
- self.CONFIGSPEC = configspec_to_str_list(self.CONFIGSPEC) # pylint: disable=C0103,C0301
+ # pylint: disable=invalid-name
+ self.CONFIGSPEC = configspec_to_str_list(self.CONFIGSPEC)
+ # pylint: enable=invalid-name
# Run get_default_config to verify the configspec is valid
# We do this at construction to announce problems as early as possible
@@ -491,6 +498,8 @@
# This way the incoming config could be empty, or have a single value.
input_config_obj = configobj.ConfigObj(input_config_data)
config = merge_two_configs(default_config, input_config_obj)
+ # Save this for get_config
+ self._orig_config = config
if self.is_this_config_valid(config):
# Convert the string values to ints and floats
@@ -501,6 +510,13 @@
'%s \n\n\n\n %s ' % ('\n'.join(config.write()),
'\n'.join(self.CONFIGSPEC)))
+ def get_config(self):
+ """
+ Return the config that was used to init this object.
+ :return: A ConfigObj.
+ """
+ return self._orig_config
+
@classmethod
def is_this_config_valid(cls, config):
"""
@@ -541,8 +557,11 @@
log.critical('Above config must conform to this configspec: ')
pretty_str = pprint.pformat(cls.CONFIGSPEC)
+ # The isinstance check ensures there is a write method
+ # pylint: disable=no-member
if isinstance(cls.CONFIGSPEC, configobj.ConfigObj):
- pretty_str = pprint.pformat(cls.CONFIGSPEC.write()) # pylint: disable=no-member,line-too-long
+ pretty_str = pprint.pformat(cls.CONFIGSPEC.write())
+ # pylint: enable=no-member
log.critical('\n' + pretty_str)
if len(extra_items) > 0:
log.critical('Extra (misspelled?) items: ')
@@ -561,6 +580,7 @@
"""
configspec = configspec_to_str_list(cls.CONFIGSPEC)
+ log = logging.getLogger(LOG_NAME)
try:
config = configobj.ConfigObj(configspec=configspec)
validator = validate.Validator()
@@ -574,7 +594,7 @@
# by the above code means the config was bad, and we should
# stop and print out useful debugging info.
pretty_str = pprint.pformat(configspec)
- print ('This configspec failed: \n%s ', pretty_str)
- print (exc)
+ log.error('This configspec failed: \n%s ', pretty_str)
+ log.error(exc)
raise ConfigurationError(exc.message)
return config
diff --git a/wireless_automation/aspects/measurement_list.py b/wireless_automation/aspects/measurement_list.py
index 7735853..be8172c 100644
--- a/wireless_automation/aspects/measurement_list.py
+++ b/wireless_automation/aspects/measurement_list.py
@@ -13,8 +13,7 @@
class Measurement(object):
- # disable to few public methods
- # pylint: disable=R0903
+ # pylint: disable=too-few-public-methods
"""
Holds all the bits about one measurement
"""
@@ -56,5 +55,5 @@
str_list = [str(x) for x in self._global_measurement_list]
return "\n".join(str_list)
-
-TheMeasurementList = MeasurementList() # pylint: disable=invalid-name
+# pylint: disable=invalid-name
+TheMeasurementList = MeasurementList()
