diff options
author | Ryan Harper <ryan.harper@canonical.com> | 2017-12-06 16:30:22 -0600 |
---|---|---|
committer | Scott Moser <smoser@brickies.net> | 2017-12-07 22:40:07 -0500 |
commit | a110e483e8644ab73e69853ea11b6c4c6cfa04b6 (patch) | |
tree | 455267ae3a8b69d0d6d93c142160422925ba04ec | |
parent | 0cf6db3617e0cebeb89c4809396f84360827e96c (diff) | |
download | vyos-cloud-init-a110e483e8644ab73e69853ea11b6c4c6cfa04b6.tar.gz vyos-cloud-init-a110e483e8644ab73e69853ea11b6c4c6cfa04b6.zip |
pylint: Update pylint to 1.7.1, run on tests/ and tools and fix complaints.
The motivation for this is that
a.) 1.7.1 runs with python 3.6 (bionic)
b.) we want to run pylint on tests/ and tools for the same reasons
that we want to run it on cloudinit/
The changes are described below.
- Update tox.ini to invoke pylint v1.7.1.
- Modify .pylintrc generated-members ignore mocked object members (m_.*)
- Replace "dangerous" params defaulting to {}
- Fix up cloud_tests use of platforms
- Cast some instance objects to with dict()
- Handle python2.7 vs 3+ ConfigParser use of readfp (deprecated)
- Update use of assertEqual(<boolean>, value) to assert<Boolean>(value)
- replace depricated assertRegexp -> assertRegex
- Remove useless test-class calls to super class
- Assign class property accessors a result and use it
- Fix missing class member in CepkoResultTests
- Fix Cheetah test import
29 files changed, 121 insertions, 266 deletions
@@ -56,5 +56,5 @@ ignored-classes=optparse.Values,thread._local # List of members which are set dynamically and missed by pylint inference # system, and so shouldn't trigger E1101 when accessed. Python regular # expressions are accepted. -generated-members=types,http.client,command_handlers +generated-members=types,http.client,command_handlers,m_.* diff --git a/cloudinit/cmd/tests/test_clean.py b/cloudinit/cmd/tests/test_clean.py index af438aab..1379740b 100644 --- a/cloudinit/cmd/tests/test_clean.py +++ b/cloudinit/cmd/tests/test_clean.py @@ -151,7 +151,7 @@ class TestClean(CiTestCase): 'sys.argv': {'new': ['clean', '--logs']}}, clean.main) - self.assertEqual(0, context_manager.exception.code) + self.assertRaisesCodeEqual(0, context_manager.exception.code) self.assertFalse( os.path.exists(self.log1), 'Unexpected log {0}'.format(self.log1)) diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py index 8ec9b5bc..6d4a11e8 100644 --- a/cloudinit/cmd/tests/test_status.py +++ b/cloudinit/cmd/tests/test_status.py @@ -347,7 +347,7 @@ class TestStatus(CiTestCase): '_is_cloudinit_disabled': (False, ''), 'Init': {'side_effect': self.init_class}}, status.main) - self.assertEqual(0, context_manager.exception.code) + self.assertRaisesCodeEqual(0, context_manager.exception.code) self.assertEqual('status: running\n', m_stdout.getvalue()) # vi: ts=4 expandtab syntax=python diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index feb884ab..0080c729 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -19,6 +19,11 @@ try: except ImportError: from contextlib2 import ExitStack +try: + from configparser import ConfigParser +except ImportError: + from ConfigParser import ConfigParser + from cloudinit import helpers as ch from cloudinit import util @@ -113,6 +118,16 @@ class TestCase(unittest2.TestCase): self.addCleanup(m.stop) setattr(self, attr, p) + # prefer python3 read_file over readfp but allow fallback + def parse_and_read(self, contents): + parser = ConfigParser() + if hasattr(parser, 'read_file'): + parser.read_file(contents) + elif hasattr(parser, 'readfp'): + # pylint: disable=W1505 + parser.readfp(contents) + return parser + class CiTestCase(TestCase): """This is the preferred test case base class unless user @@ -158,6 +173,18 @@ class CiTestCase(TestCase): dir = self.tmp_dir() return os.path.normpath(os.path.abspath(os.path.join(dir, path))) + def assertRaisesCodeEqual(self, expected, found): + """Handle centos6 having different context manager for assertRaises. + with assertRaises(Exception) as e: + raise Exception("BOO") + + centos6 will have e.exception as an integer. + anything nwere will have it as something with a '.code'""" + if isinstance(found, int): + self.assertEqual(expected, found) + else: + self.assertEqual(expected, found.code) + class ResourceUsingTestCase(CiTestCase): @@ -395,4 +422,12 @@ if not hasattr(mock.Mock, 'assert_not_called'): mock.Mock.assert_not_called = __mock_assert_not_called +# older unittest2.TestCase (centos6) do not have assertRaisesRegex +# And setting assertRaisesRegex to assertRaisesRegexp causes +# https://github.com/PyCQA/pylint/issues/1653 . So the workaround. +if not hasattr(unittest2.TestCase, 'assertRaisesRegex'): + def _tricky(*args, **kwargs): + return unittest2.TestCase.assertRaisesRegexp + unittest2.TestCase.assertRaisesRegex = _tricky + # vi: ts=4 expandtab diff --git a/tests/cloud_tests/__init__.py b/tests/cloud_tests/__init__.py index 98c1d6c7..dd436989 100644 --- a/tests/cloud_tests/__init__.py +++ b/tests/cloud_tests/__init__.py @@ -10,6 +10,12 @@ TESTCASES_DIR = os.path.join(BASE_DIR, 'testcases') TEST_CONF_DIR = os.path.join(BASE_DIR, 'testcases') TREE_BASE = os.sep.join(BASE_DIR.split(os.sep)[:-2]) +# This domain contains reverse lookups for hostnames that are used. +# The primary reason is so sudo will return quickly when it attempts +# to look up the hostname. i9n is just short for 'integration'. +# see also bug 1730744 for why we had to do this. +CI_DOMAIN = "i9n.cloud-init.io" + def _initialize_logging(): """Configure logging for cloud_tests.""" diff --git a/tests/cloud_tests/bddeb.py b/tests/cloud_tests/bddeb.py index c259dfea..a6d5069f 100644 --- a/tests/cloud_tests/bddeb.py +++ b/tests/cloud_tests/bddeb.py @@ -8,8 +8,7 @@ import tempfile from cloudinit import util as c_util from tests.cloud_tests import (config, LOG) -from tests.cloud_tests.platforms import (platforms, images, snapshots, - instances) +from tests.cloud_tests import platforms from tests.cloud_tests.stage import (PlatformComponent, run_stage, run_single) pre_reqs = ['devscripts', 'equivs', 'git', 'tar'] @@ -85,18 +84,18 @@ def setup_build(args): # set up image LOG.info('acquiring image for os: %s', args.build_os) img_conf = config.load_os_config(platform.platform_name, args.build_os) - image_call = partial(images.get_image, platform, img_conf) + image_call = partial(platforms.get_image, platform, img_conf) with PlatformComponent(image_call) as image: # set up snapshot - snapshot_call = partial(snapshots.get_snapshot, image) + snapshot_call = partial(platforms.get_snapshot, image) with PlatformComponent(snapshot_call) as snapshot: # create instance with cloud-config to set it up LOG.info('creating instance to build deb in') empty_cloud_config = "#cloud-config\n{}" instance_call = partial( - instances.get_instance, snapshot, empty_cloud_config, + platforms.get_instance, snapshot, empty_cloud_config, use_desc='build cloud-init deb') with PlatformComponent(instance_call) as instance: diff --git a/tests/cloud_tests/collect.py b/tests/cloud_tests/collect.py index db5ee99f..4805cea1 100644 --- a/tests/cloud_tests/collect.py +++ b/tests/cloud_tests/collect.py @@ -64,9 +64,9 @@ def collect_test_data(args, snapshot, os_name, test_name): # skip the testcase with a warning req_features = test_config.get('required_features', []) if any(feature not in snapshot.features for feature in req_features): - LOG.warn('test config %s requires features not supported by image, ' - 'skipping.\nrequired features: %s\nsupported features: %s', - test_name, req_features, snapshot.features) + LOG.warning('test config %s requires features not supported by image, ' + 'skipping.\nrequired features: %s\nsupported features: %s', + test_name, req_features, snapshot.features) return ({}, 0) # if there are user data overrides required for this test case, apply them diff --git a/tests/cloud_tests/config.py b/tests/cloud_tests/config.py index 52fc2bda..8bd569fd 100644 --- a/tests/cloud_tests/config.py +++ b/tests/cloud_tests/config.py @@ -92,7 +92,7 @@ def load_platform_config(platform_name, require_enabled=False): def load_os_config(platform_name, os_name, require_enabled=False, - feature_overrides={}): + feature_overrides=None): """Load configuration for os. @param platform_name: platform name to load os config for @@ -101,6 +101,8 @@ def load_os_config(platform_name, os_name, require_enabled=False, @param feature_overrides: feature flag overrides to merge with features @return_value: config dict """ + if feature_overrides is None: + feature_overrides = {} main_conf = c_util.read_conf(RELEASES_CONF) default = main_conf['default_release_config'] image = main_conf['releases'][os_name] diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index 1706f59b..1c5b5405 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -12,7 +12,8 @@ from cloudinit import util as c_util class CloudTestCase(unittest.TestCase): """Base test class for verifiers.""" - data = None + # data gets populated in get_suite.setUpClass + data = {} conf = None _cloud_config = None diff --git a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py index eb6f0650..a405b30b 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py +++ b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py @@ -1,7 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. """cloud-init Integration Test Verify Script.""" -from tests.cloud_tests.instances.nocloudkvm import CI_DOMAIN +from tests.cloud_tests import CI_DOMAIN from tests.cloud_tests.testcases import base diff --git a/tests/cloud_tests/util.py b/tests/cloud_tests/util.py index c5cd6974..2aedcd0d 100644 --- a/tests/cloud_tests/util.py +++ b/tests/cloud_tests/util.py @@ -262,7 +262,7 @@ def shell_safe(cmd): out = subprocess.check_output( ["getopt", "--shell", "sh", "--options", "", "--", "--"] + list(cmd)) # out contains ' -- <data>\n'. drop the ' -- ' and the '\n' - return out[4:-1].decode() + return out.decode()[4:-1] def shell_pack(cmd): diff --git a/tests/unittests/test_cs_util.py b/tests/unittests/test_cs_util.py index ee88520d..2a1095b9 100644 --- a/tests/unittests/test_cs_util.py +++ b/tests/unittests/test_cs_util.py @@ -35,6 +35,7 @@ class CepkoMock(Cepko): # touched the underlying Cepko class methods. class CepkoResultTests(test_helpers.TestCase): def setUp(self): + self.c = Cepko() raise test_helpers.SkipTest('This test is completely useless') def test_getitem(self): diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 226c214a..5ab48897 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -36,9 +36,9 @@ def construct_valid_ovf_env(data=None, pubkeys=None, userdata=None): """ for key, dval in data.items(): if isinstance(dval, dict): - val = dval.get('text') - attrs = ' ' + ' '.join(["%s='%s'" % (k, v) for k, v in dval.items() - if k != 'text']) + val = dict(dval).get('text') + attrs = ' ' + ' '.join(["%s='%s'" % (k, v) for k, v + in dict(dval).items() if k != 'text']) else: val = dval attrs = "" @@ -897,9 +897,6 @@ class TestCanDevBeReformatted(CiTestCase): setattr(self, sattr, patcher.start()) self.addCleanup(patcher.stop) - def setUp(self): - super(TestCanDevBeReformatted, self).setUp() - def patchup(self, devs): bypath = {} for path, data in devs.items(): @@ -954,14 +951,14 @@ class TestCanDevBeReformatted(CiTestCase): '/dev/sda3': {'num': 3}, }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertFalse(False, value) + self.assertFalse(value) self.assertIn("3 or more", msg.lower()) def test_no_partitions_is_false(self): """A disk with no partitions can not be formatted.""" self.patchup({'/dev/sda': {}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertEqual(False, value) + self.assertFalse(value) self.assertIn("not partitioned", msg.lower()) def test_two_partitions_not_ntfs_false(self): @@ -973,7 +970,7 @@ class TestCanDevBeReformatted(CiTestCase): '/dev/sda2': {'num': 2, 'fs': 'ext4', 'files': []}, }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertFalse(False, value) + self.assertFalse(value) self.assertIn("not ntfs", msg.lower()) def test_two_partitions_ntfs_populated_false(self): @@ -986,7 +983,7 @@ class TestCanDevBeReformatted(CiTestCase): 'files': ['secret.txt']}, }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertFalse(False, value) + self.assertFalse(value) self.assertIn("files on it", msg.lower()) def test_two_partitions_ntfs_empty_is_true(self): @@ -998,7 +995,7 @@ class TestCanDevBeReformatted(CiTestCase): '/dev/sda2': {'num': 2, 'fs': 'ntfs', 'files': []}, }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertEqual(True, value) + self.assertTrue(value) self.assertIn("safe for", msg.lower()) def test_one_partition_not_ntfs_false(self): @@ -1009,7 +1006,7 @@ class TestCanDevBeReformatted(CiTestCase): '/dev/sda1': {'num': 1, 'fs': 'zfs'}, }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertEqual(False, value) + self.assertFalse(value) self.assertIn("not ntfs", msg.lower()) def test_one_partition_ntfs_populated_false(self): @@ -1021,7 +1018,7 @@ class TestCanDevBeReformatted(CiTestCase): 'files': ['file1.txt', 'file2.exe']}, }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertEqual(False, value) + self.assertFalse(value) self.assertIn("files on it", msg.lower()) def test_one_partition_ntfs_empty_is_true(self): @@ -1032,7 +1029,7 @@ class TestCanDevBeReformatted(CiTestCase): '/dev/sda1': {'num': 1, 'fs': 'ntfs', 'files': []} }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertEqual(True, value) + self.assertTrue(value) self.assertIn("safe for", msg.lower()) def test_one_partition_ntfs_empty_with_dataloss_file_is_true(self): @@ -1044,7 +1041,7 @@ class TestCanDevBeReformatted(CiTestCase): 'files': ['dataloss_warning_readme.txt']} }}}) value, msg = dsaz.can_dev_be_reformatted("/dev/sda") - self.assertEqual(True, value) + self.assertTrue(value) self.assertIn("safe for", msg.lower()) def test_one_partition_through_realpath_is_true(self): @@ -1059,7 +1056,7 @@ class TestCanDevBeReformatted(CiTestCase): 'realpath': '/dev/sdb1'} }}}) value, msg = dsaz.can_dev_be_reformatted(epath) - self.assertEqual(True, value) + self.assertTrue(value) self.assertIn("safe for", msg.lower()) def test_three_partition_through_realpath_is_false(self): @@ -1078,7 +1075,7 @@ class TestCanDevBeReformatted(CiTestCase): 'realpath': '/dev/sdb3'} }}}) value, msg = dsaz.can_dev_be_reformatted(epath) - self.assertEqual(False, value) + self.assertFalse(value) self.assertIn("3 or more", msg.lower()) diff --git a/tests/unittests/test_datasource/test_digitalocean.py b/tests/unittests/test_datasource/test_digitalocean.py index ec321733..3127014b 100644 --- a/tests/unittests/test_datasource/test_digitalocean.py +++ b/tests/unittests/test_datasource/test_digitalocean.py @@ -199,9 +199,8 @@ class TestDataSourceDigitalOcean(CiTestCase): class TestNetworkConvert(CiTestCase): - @mock.patch('cloudinit.net.get_interfaces_by_mac') - def _get_networking(self, m_get_by_mac): - m_get_by_mac.return_value = { + def _get_networking(self): + self.m_get_by_mac.return_value = { '04:01:57:d1:9e:01': 'ens1', '04:01:57:d1:9e:02': 'ens2', 'b8:ae:ed:75:5f:9a': 'enp0s25', @@ -211,6 +210,10 @@ class TestNetworkConvert(CiTestCase): self.assertIn('config', netcfg) return netcfg + def setUp(self): + super(TestNetworkConvert, self).setUp() + self.add_patch('cloudinit.net.get_interfaces_by_mac', 'm_get_by_mac') + def test_networking_defined(self): netcfg = self._get_networking() self.assertIsNotNone(netcfg) diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index ba042eac..f0dc8338 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -330,7 +330,8 @@ class TestEc2(test_helpers.HttprettyTestCase): ds.fallback_nic = 'eth9' with mock.patch(get_interface_mac_path) as m_get_interface_mac: m_get_interface_mac.return_value = mac1 - ds.network_config # Will re-crawl network metadata + nc = ds.network_config # Will re-crawl network metadata + self.assertIsNotNone(nc) self.assertIn('Re-crawl of metadata service', self.logs.getvalue()) expected = {'version': 1, 'config': [ {'mac_address': '06:17:04:d7:26:09', diff --git a/tests/unittests/test_distros/test_create_users.py b/tests/unittests/test_distros/test_create_users.py index aa13670a..5670904a 100644 --- a/tests/unittests/test_distros/test_create_users.py +++ b/tests/unittests/test_distros/test_create_users.py @@ -7,7 +7,11 @@ from cloudinit.tests.helpers import (TestCase, mock) class MyBaseDistro(distros.Distro): # MyBaseDistro is here to test base Distro class implementations - def __init__(self, name="basedistro", cfg={}, paths={}): + def __init__(self, name="basedistro", cfg=None, paths=None): + if not cfg: + cfg = {} + if not paths: + paths = {} super(MyBaseDistro, self).__init__(name, cfg, paths) def install_packages(self, pkglist): @@ -42,7 +46,6 @@ class MyBaseDistro(distros.Distro): @mock.patch("cloudinit.distros.util.subp") class TestCreateUser(TestCase): def setUp(self): - super(TestCase, self).setUp() self.dist = MyBaseDistro() def _useradd2call(self, args): diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index c4bd11bc..8d0b2634 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -188,9 +188,6 @@ hn0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500 status: active """ - def setUp(self): - super(TestNetCfgDistro, self).setUp() - def _get_distro(self, dname, renderers=None): cls = distros.fetch(dname) cfg = settings.CFG_BUILTIN diff --git a/tests/unittests/test_handler/test_handler_lxd.py b/tests/unittests/test_handler/test_handler_lxd.py index e0d9ab6c..a2054980 100644 --- a/tests/unittests/test_handler/test_handler_lxd.py +++ b/tests/unittests/test_handler/test_handler_lxd.py @@ -25,9 +25,6 @@ class TestLxd(t_help.CiTestCase): } } - def setUp(self): - super(TestLxd, self).setUp() - def _get_cloud(self, distro): cls = distros.fetch(distro) paths = helpers.Paths({}) diff --git a/tests/unittests/test_handler/test_handler_power_state.py b/tests/unittests/test_handler/test_handler_power_state.py index 85a0fe0a..3c726422 100644 --- a/tests/unittests/test_handler/test_handler_power_state.py +++ b/tests/unittests/test_handler/test_handler_power_state.py @@ -9,9 +9,6 @@ from cloudinit.tests.helpers import mock class TestLoadPowerState(t_help.TestCase): - def setUp(self): - super(self.__class__, self).setUp() - def test_no_config(self): # completely empty config should mean do nothing (cmd, _timeout, _condition) = psc.load_power_state({}) diff --git a/tests/unittests/test_handler/test_handler_yum_add_repo.py b/tests/unittests/test_handler/test_handler_yum_add_repo.py index b7adbe50..b90a3af3 100644 --- a/tests/unittests/test_handler/test_handler_yum_add_repo.py +++ b/tests/unittests/test_handler/test_handler_yum_add_repo.py @@ -5,10 +5,6 @@ from cloudinit import util from cloudinit.tests import helpers -try: - from configparser import ConfigParser -except ImportError: - from ConfigParser import ConfigParser import logging import shutil from six import StringIO @@ -58,8 +54,7 @@ class TestConfig(helpers.FilesystemMockingTestCase): self.patchUtils(self.tmp) cc_yum_add_repo.handle('yum_add_repo', cfg, None, LOG, []) contents = util.load_file("/etc/yum.repos.d/epel_testing.repo") - parser = ConfigParser() - parser.readfp(StringIO(contents)) + parser = self.parse_and_read(StringIO(contents)) expected = { 'epel_testing': { 'name': 'Extra Packages for Enterprise Linux 5 - Testing', @@ -95,8 +90,7 @@ class TestConfig(helpers.FilesystemMockingTestCase): self.patchUtils(self.tmp) cc_yum_add_repo.handle('yum_add_repo', cfg, None, LOG, []) contents = util.load_file("/etc/yum.repos.d/puppetlabs_products.repo") - parser = ConfigParser() - parser.readfp(StringIO(contents)) + parser = self.parse_and_read(StringIO(contents)) expected = { 'puppetlabs_products': { 'name': 'Puppet Labs Products El 6 - $basearch', diff --git a/tests/unittests/test_handler/test_handler_zypper_add_repo.py b/tests/unittests/test_handler/test_handler_zypper_add_repo.py index 315c2a5e..72ab6c08 100644 --- a/tests/unittests/test_handler/test_handler_zypper_add_repo.py +++ b/tests/unittests/test_handler/test_handler_zypper_add_repo.py @@ -9,10 +9,6 @@ from cloudinit import util from cloudinit.tests import helpers from cloudinit.tests.helpers import mock -try: - from configparser import ConfigParser -except ImportError: - from ConfigParser import ConfigParser import logging from six import StringIO @@ -70,8 +66,7 @@ class TestConfig(helpers.FilesystemMockingTestCase): root_d = self.tmp_dir() cc_zypper_add_repo._write_repos(cfg['repos'], root_d) contents = util.load_file("%s/testing-foo.repo" % root_d) - parser = ConfigParser() - parser.readfp(StringIO(contents)) + parser = self.parse_and_read(StringIO(contents)) expected = { 'testing-foo': { 'name': 'test-foo', diff --git a/tests/unittests/test_reporting.py b/tests/unittests/test_reporting.py index 571420ed..e15ba6cf 100644 --- a/tests/unittests/test_reporting.py +++ b/tests/unittests/test_reporting.py @@ -126,7 +126,7 @@ class TestBaseReportingHandler(TestCase): def test_base_reporting_handler_is_abstract(self): regexp = r".*abstract.*publish_event.*" - self.assertRaisesRegexp(TypeError, regexp, handlers.ReportingHandler) + self.assertRaisesRegex(TypeError, regexp, handlers.ReportingHandler) class TestLogHandler(TestCase): diff --git a/tests/unittests/test_templating.py b/tests/unittests/test_templating.py index b911d929..53154d33 100644 --- a/tests/unittests/test_templating.py +++ b/tests/unittests/test_templating.py @@ -14,7 +14,7 @@ from cloudinit import templater try: import Cheetah HAS_CHEETAH = True - Cheetah # make pyflakes happy, as Cheetah is not used here + c = Cheetah # make pyflakes and pylint happy, as Cheetah is not used here except ImportError: HAS_CHEETAH = False diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 71f59529..787ca208 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -695,9 +695,9 @@ class TestSubp(helpers.CiTestCase): util.write_file(noshebang, 'true\n') os.chmod(noshebang, os.stat(noshebang).st_mode | stat.S_IEXEC) - self.assertRaisesRegexp(util.ProcessExecutionError, - 'Missing #! in script\?', - util.subp, (noshebang,)) + self.assertRaisesRegex(util.ProcessExecutionError, + 'Missing #! in script\?', + util.subp, (noshebang,)) def test_returns_none_if_no_capture(self): (out, err) = util.subp(self.stdin2out, data=b'', capture=False) diff --git a/tests/unittests/test_vmware_config_file.py b/tests/unittests/test_vmware_config_file.py index 808d303a..0f8cda95 100644 --- a/tests/unittests/test_vmware_config_file.py +++ b/tests/unittests/test_vmware_config_file.py @@ -133,7 +133,8 @@ class TestVmwareConfigFile(CiTestCase): conf = Config(cf) with self.assertRaises(ValueError): - conf.reset_password() + pw = conf.reset_password + self.assertIsNone(pw) cf.clear() cf._insertKey("PASSWORD|RESET", "yes") diff --git a/tools/hacking.py b/tools/hacking.py deleted file mode 100755 index e6a05136..00000000 --- a/tools/hacking.py +++ /dev/null @@ -1,172 +0,0 @@ -#!/usr/bin/env python -# Copyright (c) 2012, Cloudscaling -# 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. - -"""cloudinit HACKING file compliance testing (based off of nova hacking.py) - -built on top of pep8.py -""" - -import inspect -import logging -import re -import sys - -import pep8 - -# Don't need this for testing -logging.disable('LOG') - -# N1xx comments -# N2xx except -# N3xx imports -# N4xx docstrings -# N[5-9]XX (future use) - -DOCSTRING_TRIPLE = ['"""', "'''"] -VERBOSE_MISSING_IMPORT = False -_missingImport = set([]) - - -def import_normalize(line): - # convert "from x import y" to "import x.y" - # handle "from x import y as z" to "import x.y as z" - split_line = line.split() - if (line.startswith("from ") and "," not in line and - split_line[2] == "import" and split_line[3] != "*" and - split_line[1] != "__future__" and - (len(split_line) == 4 or (len(split_line) == 6 and - split_line[4] == "as"))): - return "import %s.%s" % (split_line[1], split_line[3]) - else: - return line - - -def cloud_import_alphabetical(physical_line, line_number, lines): - """Check for imports in alphabetical order. - - HACKING guide recommendation for imports: - imports in human alphabetical order - N306 - """ - # handle import x - # use .lower since capitalization shouldn't dictate order - split_line = import_normalize(physical_line.strip()).lower().split() - split_previous = import_normalize(lines[line_number - 2]) - split_previous = split_previous.strip().lower().split() - # with or without "as y" - length = [2, 4] - if (len(split_line) in length and len(split_previous) in length and - split_line[0] == "import" and split_previous[0] == "import"): - if split_line[1] < split_previous[1]: - return (0, "N306: imports not in alphabetical order (%s, %s)" - % (split_previous[1], split_line[1])) - - -def cloud_docstring_start_space(physical_line): - """Check for docstring not start with space. - - HACKING guide recommendation for docstring: - Docstring should not start with space - N401 - """ - pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start - if (pos != -1 and len(physical_line) > pos + 1): - if (physical_line[pos + 3] == ' '): - return (pos, - "N401: one line docstring should not start with a space") - - -def cloud_todo_format(physical_line): - """Check for 'TODO()'. - - HACKING guide recommendation for TODO: - Include your name with TODOs as in "#TODO(termie)" - N101 - """ - pos = physical_line.find('TODO') - pos1 = physical_line.find('TODO(') - pos2 = physical_line.find('#') # make sure it's a comment - if (pos != pos1 and pos2 >= 0 and pos2 < pos): - return pos, "N101: Use TODO(NAME)" - - -def cloud_docstring_one_line(physical_line): - """Check one line docstring end. - - HACKING guide recommendation for one line docstring: - A one line docstring looks like this and ends in a period. - N402 - """ - pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start - end = max([physical_line[-4:-1] == i for i in DOCSTRING_TRIPLE]) # end - if (pos != -1 and end and len(physical_line) > pos + 4): - if (physical_line[-5] != '.'): - return pos, "N402: one line docstring needs a period" - - -def cloud_docstring_multiline_end(physical_line): - """Check multi line docstring end. - - HACKING guide recommendation for docstring: - Docstring should end on a new line - N403 - """ - pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start - if (pos != -1 and len(physical_line) == pos): - print(physical_line) - if (physical_line[pos + 3] == ' '): - return (pos, "N403: multi line docstring end on new line") - - -current_file = "" - - -def readlines(filename): - """Record the current file being tested.""" - pep8.current_file = filename - return open(filename).readlines() - - -def add_cloud(): - """Monkey patch pep8 for cloud-init guidelines. - - Look for functions that start with cloud_ - and add them to pep8 module. - - Assumes you know how to write pep8.py checks - """ - for name, function in globals().items(): - if not inspect.isfunction(function): - continue - if name.startswith("cloud_"): - exec("pep8.%s = %s" % (name, name)) - - -if __name__ == "__main__": - # NOVA based 'hacking.py' error codes start with an N - pep8.ERRORCODE_REGEX = re.compile(r'[EWN]\d{3}') - add_cloud() - pep8.current_file = current_file - pep8.readlines = readlines - try: - pep8._main() - finally: - if len(_missingImport) > 0: - sys.stderr.write( - "%i imports missing in this test environment\n" % - len(_missingImport)) - -# vi: ts=4 expandtab diff --git a/tools/make-mime.py b/tools/make-mime.py index f6a72044..d321479b 100755 --- a/tools/make-mime.py +++ b/tools/make-mime.py @@ -23,7 +23,7 @@ def file_content_type(text): filename, content_type = text.split(":", 1) return (open(filename, 'r'), filename, content_type.strip()) except ValueError: - raise argparse.ArgumentError("Invalid value for %r" % (text)) + raise argparse.ArgumentError(text, "Invalid value for %r" % (text)) def main(): diff --git a/tools/mock-meta.py b/tools/mock-meta.py index a5d14ab7..724f7fc4 100755 --- a/tools/mock-meta.py +++ b/tools/mock-meta.py @@ -17,6 +17,7 @@ Then: ec2metadata --instance-id """ +import argparse import functools import json import logging @@ -27,8 +28,6 @@ import string import sys import yaml -from optparse import OptionParser - try: from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler import httplib as hclient @@ -415,29 +414,27 @@ def setup_logging(log_level, fmt='%(levelname)s: @%(name)s : %(message)s'): def extract_opts(): - parser = OptionParser() - parser.add_option("-p", "--port", dest="port", action="store", type=int, - default=80, metavar="PORT", - help=("port from which to serve traffic" - " (default: %default)")) - parser.add_option("-a", "--addr", dest="address", action="store", type=str, - default='::', metavar="ADDRESS", - help=("address from which to serve traffic" - " (default: %default)")) - parser.add_option("-f", '--user-data-file', dest='user_data_file', - action='store', metavar='FILE', - help=("user data filename to serve back to" - "incoming requests")) - (options, args) = parser.parse_args() - out = dict() - out['extra'] = args - out['port'] = options.port - out['user_data_file'] = None - out['address'] = options.address - if options.user_data_file: - if not os.path.isfile(options.user_data_file): + parser = argparse.ArgumentParser() + parser.add_argument("-p", "--port", dest="port", action="store", type=int, + default=80, metavar="PORT", + help=("port from which to serve traffic" + " (default: %default)")) + parser.add_argument("-a", "--addr", dest="address", action="store", + type=str, default='::', metavar="ADDRESS", + help=("address from which to serve traffic" + " (default: %default)")) + parser.add_argument("-f", '--user-data-file', dest='user_data_file', + action='store', metavar='FILE', + help=("user data filename to serve back to" + "incoming requests")) + parser.add_argument('extra', nargs='*') + args = parser.parse_args() + out = {'port': args.port, 'address': args.address, 'extra': args.extra, + 'user_data_file': None} + if args.user_data_file: + if not os.path.isfile(args.user_data_file): parser.error("Option -f specified a non-existent file") - with open(options.user_data_file, 'rb') as fh: + with open(args.user_data_file, 'rb') as fh: out['user_data_file'] = fh.read() return out @@ -21,12 +21,13 @@ setenv = LC_ALL = en_US.utf-8 [testenv:pylint] +basepython = python3 deps = # requirements pylint==1.7.1 # test-requirements because unit tests are now present in cloudinit tree -r{toxinidir}/test-requirements.txt -commands = {envpython} -m pylint {posargs:cloudinit} +commands = {envpython} -m pylint {posargs:cloudinit tests tools} [testenv:py3] basepython = python3 @@ -119,7 +120,7 @@ commands = {envpython} -m pyflakes {posargs:cloudinit/ tests/ tools/} deps = pyflakes [testenv:tip-pylint] -commands = {envpython} -m pylint {posargs:cloudinit} +commands = {envpython} -m pylint {posargs:cloudinit tests tools} deps = # requirements pylint |